Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Makefile #99

Open
wants to merge 1 commit into
base: public
Choose a base branch
from
Open

Conversation

bakatrouble
Copy link
Contributor

Makefile templates are missing video/VideoPacketSender and video/VideoFEC includes

@zevlg
Copy link
Contributor

zevlg commented Oct 25, 2019

This PR fixes #96

Copy link

@andy-shev andy-shev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makefile.in mustn't be part of repository. (Means you should drop that change from the commit, and leave only the proper one, i.e. in Makefile.am)

@mymedia2
Copy link
Contributor

mymedia2 commented Jan 6, 2021

@andy-shev

Makefile.in mustn't be part of repository

That's not dogma. There are several approaches to Autotools with version control systems. See comparisons:

  1. https://www.gnu.org/software/automake/manual/automake.html#CVS
  2. https://www.gnu.org/software/gettext/manual/gettext.html#Files-under-Version-Control

@grishka
Copy link
Owner

grishka commented Jan 6, 2021

I've seen many repositories include configure. This way you don't need to run anything to generate it and don't need any build-time dependencies.

@andy-shev
Copy link

andy-shev commented Jan 7, 2021

I've seen many repositories include configure. This way you don't need to run anything to generate it and don't need any build-time dependencies.

If they are all wrong, why should be you? https://en.wikipedia.org/wiki/Cargo_cult_programming

And to my point: VCS and release tarball are different in this case. when you build your release with make dist (https://www.gnu.org/software/automake/manual/html_node/Preparing-Distributions.html) you will get everything needed. Simply learn your tools (as I already mentioned somewhere else in this repo) :-)

@grishka
Copy link
Owner

grishka commented Jan 7, 2021

The idea is that you clone the repo, run ./configure && make and that's it. Including configure is simply making it more convenient for your users by not requiring them to make sense of your build system. And making my project convenient for my users is my number one priority.

@andy-shev
Copy link

andy-shev commented Jan 7, 2021

The idea is that you clone the repo, run ./configure && make and that's it. Including configure is simply making it more convenient for your users by not requiring them to make sense of your build system. And making my project convenient for my users is my number one priority.

I have updates my comment above. For your convenience repeating here the additional part:

And to my point: VCS and release tarball are different in this case. when you build your release with make dist (https://www.gnu.org/software/automake/manual/html_node/Preparing-Distributions.html) you will get everything needed. Simply learn your tools (as I already mentioned somewhere else in this repo) :-)

To @mymedia2: See above as well. Feel free to ask additional questions if you have something to be cleared.

@mymedia2
Copy link
Contributor

mymedia2 commented Jan 7, 2021

@andy-shev but your link says nothing about version control systems. Why do not you admit a workflow with generated files in Git? It is discussed in Automake docs.

@andy-shev
Copy link

@andy-shev but your link says nothing about version control systems. Why do not you admit a workflow with generated files in Git? It is discussed in Automake docs.

It's suboptimal to have generated files under VCS. User will regenerate them anyway (you never know what the actual version of autotools on the user's side). I see only disadvantages of this. And on top of that the standard de facto is to produce releases. If you do tags in VCS it's user's responsibility to generate necessary files on their side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants