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

Couple of ideas to further improve the builder #38

Merged
merged 2 commits into from
Jul 9, 2018

Conversation

Dorthu
Copy link
Member

@Dorthu Dorthu commented Jul 4, 2018

I really like what @ginkgomzd with the build process - I'm not going to
lie, this is my first Makefile, and I appreciate the cleanup. I had a
few ideas looking it over:

If we're going to build a package during the install process,
why not install the package that we just built? I think that works
better anyway - once you test the package that was installed, you can
confidently distribute it knowing that it is the same code you just ran.

I also added a clean step to delete all of the generated files
(linodecli/data-* and linode-cli.sh along with the contents of dist/) to
make sure everything's fresh with each build.

I really like what @ginkgomzd with the build process - I'm not going to
lie, this is my first Makefile, and I appreciate the cleanup.  I had a
few ideas looking it over:

If we're going to build a package during the install process,
why not install the package that we just built?  I think that works
better anyway - once you test the package that was installed, you can
confidently distribute it knowing that it is the same code you just ran.

I also  added a clean step to delete all of the generated files
(linodecli/data-* and linode-cli.sh along with the contents of dist/) to
make sure everything's fresh with each build.
@Dorthu Dorthu mentioned this pull request Jul 4, 2018
@ginkgomzd
Copy link
Contributor

Looks good.
Only thing is I don't know if this is a problem for installing alternate builds. Since install will automatically re-build, I don't know if that is taking too much control away from developers.

If the final step of build is touch build, then build will only run when being called directly (or when needed).
https://www.gnu.org/software/make/manual/html_node/Empty-Targets.html#Empty-Targets

@Dorthu
Copy link
Member Author

Dorthu commented Jul 6, 2018

That's fair, but old install behavior was a $(PYCMD) setup.py install, which does a build on its own (and ignores the package that build produces). I think this is better - at least you're installing from a package that is like what is actually distributed. Also, if the spec changes on developers.linode.com, nothing on the filesystem would've changed, and a make install would unexpectedly not rebuild the package.

@yoshizzle yoshizzle merged commit 66c6fed into master Jul 9, 2018
@er0k er0k deleted the feature/more-builder-tweaks branch May 18, 2021 15:25
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.

3 participants