-
Notifications
You must be signed in to change notification settings - Fork 133
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
bump 2.31.1 #535
Merged
Merged
bump 2.31.1 #535
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
2.31.0 | ||
2.31.1 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
remember to run the new Makefile target for validating this file which is in our runbook now.
@matthewfala @rawahars should we make that target run in the release target? that way we fail fast.
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.
Ran the script. It works.
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.
what do you think about just making it part of the
release
target so we already run it a fail fast?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.
That's a good idea. I can submit a pr for it.
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.
Actually, it's not clear what the point of this script is,
Is it supposed to fail if the following is not in JSON format?
the make validate-version-file-format does not, if that is the case.
Meanwhile it appears that the build linux plugins does fail if the version document is not JSON regardless of whether the validate-version-file-format command is run or not
Our runbook specifies:
The above changes go in the
linux.version
andwindows.versions
file. Runmake validate-version-file-format
to check that the update was successful. If formatted incorrectly, it will lead to a release pipeline failure.If the JSON file is not formatted correctly, it won't be printed out, so that may be a way to manually check for issues.
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.
@PettitWesley I think it would be. great if we add it to the
release
target.@matthewfala Yes that is correct. The make target would fail if the json files for Windows and Linux are not formatted properly. It would be a fast fail to ensure that our Pipelines do not fail due to invalid JSON.
I am not clear what you mean when you say
the make validate-version-file-format does not, if that is the case.
For example-
This would succeed since the format of json is correct.
This would fail with the following error-
This is because the above JSON is not formatted properly and has an extra comma at
"cloudwatch-plugin": "v1.9.1",
. Such errors can be easily identified before we commit the changes leading to pipeline failures.