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

Use 4th digit in version number for CI build number #541

Merged
merged 7 commits into from
Oct 16, 2023

Conversation

ASpoonPlaysGames
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Sep 4, 2023
Copy link
Member

@pg9182 pg9182 left a comment

Choose a reason for hiding this comment

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

I like this change (but haven't tested it), but please ping me before merging this.

@GeckoEidechse
Copy link
Member

Uh, no merge conflict with #542 merged but I guess this PR still needs to be updated so that run number is written to both exe and dll?

Or is this kinda done implicitly already? ^^

@ASpoonPlaysGames
Copy link
Contributor Author

Uh, no merge conflict with #542 merged but I guess this PR still needs to be updated so that run number is written to both exe and dll?

Correct

@ASpoonPlaysGames ASpoonPlaysGames removed the needs code review Changes from PR still need to be reviewed in code label Sep 16, 2023
@Alystrasz Alystrasz self-requested a review October 3, 2023 07:53
@ASpoonPlaysGames
Copy link
Contributor Author

image
another example of successful testing, going to remove the needs testing label

@ASpoonPlaysGames ASpoonPlaysGames added READY TO MERGE This mergeable right now and removed needs testing Changes from the PR still need to be tested labels Oct 14, 2023
Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Can confirm that version number is set accordingly.

$ exiftool Northstar.dll 
ExifTool Version Number         : 12.60
File Name                       : Northstar.dll
...
File Version Number             : 0.0.0.2823
Product Version Number          : 0.0.0.2823
...
$ exiftool NorthstarLauncher.exe 
ExifTool Version Number         : 12.60
File Name                       : NorthstarLauncher.exe
...
File Version Number             : 0.0.0.2823
Product Version Number          : 0.0.0.2823
...

Didn't test running game but based on the changes this should have no impact.

Code changes look fine, comment was also properly updated.
(I'm still annoyed that it's convention in C to drop the != 0 and rely on truthiness of integers but that's besides the point :P)

@GeckoEidechse GeckoEidechse merged commit d463a43 into R2Northstar:main Oct 16, 2023
2 checks passed
@ASpoonPlaysGames ASpoonPlaysGames deleted the ci-build-number branch January 4, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
READY TO MERGE This mergeable right now
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Use 4th digit in version number for CI build number
4 participants