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.

Sign up for GitHub

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

Jump to bottom

comparable_patch.sh updates for Windows #3561

Merged
merged 5 commits into from
Dec 6, 2023
Merged

comparable_patch.sh updates for Windows #3561

merged 5 commits into from
Dec 6, 2023

Conversation

andrew-m-leonard
Copy link
Contributor

@andrew-m-leonard andrew-m-leonard commented Dec 4, 2023 •
edited
Loading

Updates to improve comparable_patch.sh for Windows

  • Documentation updates
  • Allow for optionally patching VS_VERSION_INFO in use case where COMPANY_NAME is the same
  • Improve argument parsing
  • Patch for module-info.class differences due to module attribute ordering differences
  • Patch for SystemModule$*.class differences caused by differing module executables due to COMPANY_NAME differences
  • General tidy up and adding of more comments

@andrew-m-leonard
Update comparable_patch for Windows
7887254 
Signed-off-by: U-andrew-repro-te\adoptium <[email protected]>
@andrew-m-leonard andrew-m-leonard self-assigned this Dec 4, 2023
@andrew-m-leonard
Add Windows comparable patch fixes
58d5ca0 
Signed-off-by: U-andrew-repro-te\adoptium <[email protected]>
@andrew-m-leonard
Add Windows comparable patch fixes
12a053b 
Signed-off-by: U-andrew-repro-te\adoptium <[email protected]>
sxa
sxa reviewed Dec 4, 2023
View reviewed changes
tooling/ReproducibleBuilds.MD Outdated Show resolved Hide resolved
sxa
sxa reviewed Dec 4, 2023
View reviewed changes
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Question: While I appreciate it's not something that was done in this PR is there a reason the suffix of this file is .MD instead of .md? Perhaps we should consider renaming it to be consistent with the markdown suffix elsewhere in this repository.

@andrew-m-leonard
Update comparable_patch for Windows
1cf30bd 
Signed-off-by: U-andrew-repro-te\adoptium <[email protected]>
@andrew-m-leonard
Update comparable_patch for Windows
6f50b5e 
Signed-off-by: U-andrew-repro-te\adoptium <[email protected]>
@andrew-m-leonard
Copy link
Contributor Author

Question: While I appreciate it's not something that was done in this PR is there a reason the suffix of this file is .MD instead of .md? Perhaps we should consider renaming it to be consistent with the markdown suffix elsewhere in this repository.

Yep, renamed, thanks

@andrew-m-leonard andrew-m-leonard requested a review from sxa December 4, 2023 15:29
@github-actions github-actions bot added the documentation Issues that request updates to our documentation label Dec 4, 2023
steelhead31
steelhead31 approved these changes Dec 5, 2023
View reviewed changes
Copy link
Contributor

@steelhead31 steelhead31 left a comment

Choose a reason for hiding this comment

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

LGTM

sxa
sxa approved these changes Dec 5, 2023
View reviewed changes
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

"Rubber stamp" on the basis that this has been tested elsewhere and it's not critical to the production of the builds.

@andrew-m-leonard andrew-m-leonard merged commit e6bd6a1 into adoptium:master Dec 6, 2023
22 checks passed
@karianna karianna mentioned this pull request Jan 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steelhead31 steelhead31 steelhead31 approved these changes

@sxa sxa sxa approved these changes

Assignees

@andrew-m-leonard andrew-m-leonard

Labels
documentation Issues that request updates to our documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrew-m-leonard @sxa @steelhead31