-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
Update Gluster version to 7.9 #232
base: master
Are you sure you want to change the base?
Conversation
@rwaffen can we push release to 7.9 and also update forge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tsmgeek you have missed one reference on line 66 of the readme. we mention the 3.8 release being installed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smortex why do you consider this a BC break, seems a simple upgrade of the same major release to me
@attachmentgenie since the default value of a parameter is changed, it will have consequences with users who rely on this default 🤷 Another way to deal with this and avoid installing an outdated version is to not set a default and make it mandatory for the user to set it explicitly. I am not a user of this module, so I let actual maintainers decide what they want to do: keep the version the way it is, update it or change the way it is managed 😉 |
But does that not depend if they are pinning the puppet version or pulling from git directly. It's a byproduct of not updating puppet version or a gittag for so long that people ended up pulling the master branch. Ide say that as its master it should not matter what is done, only the puppet release version is important. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tsmgeek if you could squash the commits into one, than i am happy to approve this one
Update all references to 7.9
Squashed. Proposing that a minor gluster release (or any minor fixes to package) only changes patch level of puppet release (6.0.1) but if there is a move from 7->8 we go to 6.1.0. Does that seem reasonable? If no one objects to #235 I am planning to push forward for a 7->9 PR so we bring this up to date, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposing that a minor gluster release (or any minor fixes to package) only changes patch level of puppet release (6.0.1) but if there is a move from 7->8 we go to 6.1.0. Does that seem reasonable?
Changing a patch version should only fix bugs. (yes, we try to follow semver even if it is designed for versionning API and a Puppet module is way more than an API):
Patch version Z (x.y.Z | x > 0) MUST be incremented if only backwards compatible bug fixes are introduced. A bug fix is defined as an internal change that fixes incorrect behavior.
So if we change the default for a parameter this is not a bug fix, not a new features users can rely on, and definitively a breaking change. But it's not an issue to roll a new major version: it's the same process for patch, minor and major so 🤷
If no one objects to #235 I am planning to push forward for a 7->9 PR so we bring this up to date,
Yes, if version 7 is deprecated, using this as a default does not make sense.
If multiple versions are supported, it may make sense to list them in a enum and test them all in CI, but it is another story 😃
Dear @tsmgeek, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
Update default version from 7.3 to 7.9 the last in the 7.x line.
Updated documents to match default version.