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

Install from PyPI package and keep up to date by default #354

Merged
merged 3 commits into from
Jun 15, 2022

Conversation

gdubicki
Copy link
Member

@gdubicki gdubicki commented Mar 29, 2022

Pull Request (PR) description

Puppetboard developers follow semver, use pre-releses and put a lot of effort into making the PyPI (and Docker) the best way to install the app.

vcsrepo installation method should be used only by the contributors (as: developers and testers of non-releases).

This Pull Request (PR) fixes the following issues

  • The current default install method installs a non-release from a commit that is at the top of the main branch in voxpupuli/puppetboard and does not update it unless you provide a specific different revision. I would argue that this is not what a regular user would want. I think that a regular user wants to either pin a specific release OR keep puppetboard up to date from the final (stable) releases OR maybe keep up to date from the pre-releases.
  • Using with packaged puppetboard #85

@gdubicki
Copy link
Member Author

gdubicki commented Mar 29, 2022

I was inspired by the original report of #353 in voxpupuli/puppetboard#680, as I was not even able to learn what Puppetboard version was he using based on his code...

@gdubicki gdubicki force-pushed the pip_installation_method branch 2 times, most recently from f8b7b9f to 8cd78f8 Compare March 29, 2022 21:33
@gdubicki
Copy link
Member Author

Can you please check this out, @bastelfreak @smortex @kenyon?

The failing tests seem to be breaking on puppetdb, not puppetboard.

PS I know that this would need a major release as it's a backward-incompatible change but I think it's worth it.

@gdubicki gdubicki marked this pull request as ready for review March 29, 2022 21:38
@gdubicki gdubicki force-pushed the pip_installation_method branch 2 times, most recently from bdcc767 to 2212761 Compare March 29, 2022 21:47
Copy link
Member

@kenyon kenyon left a comment

Choose a reason for hiding this comment

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

Seems like a good idea to me.

@smortex
Copy link
Member

smortex commented Mar 31, 2022

The failing tests seem to be breaking on puppetdb, not puppetboard.

I just merged #352 which should also fix your issues. Can you rebase on top of it?

From your working directory:

git fetch origin       # Download the latest code we have here
git rebase origin/master # Move your commits on top of the main branch
git push -f            # Send the changes (-f is required because we re-wrote history)

@gdubicki gdubicki force-pushed the pip_installation_method branch 2 times, most recently from 8f8d353 to 48c2a6a Compare April 1, 2022 20:12
@gdubicki
Copy link
Member Author

gdubicki commented Apr 1, 2022

Done, @smortex, but I have to clean up some unnecessary changes, please hold on with merging.

Puppetboard developers follow semver, use pre-releses
and put a lot of effort into making the PyPI
(and Docker) the best way to install the app.

vcsrepo installation method should be used only
by the contributors (as: developers and testers
of non-releases).
for a given installation method
@gdubicki
Copy link
Member Author

gdubicki commented Apr 1, 2022

Ok, done.

I have also added some extra validation as over the past few days I have learned the hard way that it's very easy to not get the version you expect if you mess up the params, if you don't have such validation. 🙄

@gdubicki
Copy link
Member Author

gdubicki commented Apr 5, 2022

What do you think, @smortex, is it merge’able now?

@gdubicki
Copy link
Member Author

gdubicki commented Jun 15, 2022

@smortex , @kenyon : can I please get a quick re-review of the recent changes before I merge?

@gdubicki
Copy link
Member Author

Thanks @kenyon! ..but I actually don’t have the permissions to merge it on my own. 😞

@bastelfreak bastelfreak merged commit 2d3e850 into voxpupuli:master Jun 15, 2022
@gdubicki gdubicki deleted the pip_installation_method branch June 16, 2022 07:57
@rledousa
Copy link

THANK YOU!!!

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

Successfully merging this pull request may close these issues.

None yet

5 participants