Skip to content

#436 Create Build Action#438

Merged
lahm86 merged 2 commits intoLostArtefacts:masterfrom
lahm86:issue-436-build-action
Apr 1, 2023
Merged

#436 Create Build Action#438
lahm86 merged 2 commits intoLostArtefacts:masterfrom
lahm86:issue-436-build-action

Conversation

@lahm86
Copy link
Copy Markdown
Collaborator

@lahm86 lahm86 commented Mar 31, 2023

The proof will be in the pudding with this one, but I've tested it on a private repository and it looks OK. I'll leave the issue open in case any tweaks are needed after it runs.

The change in TR2ItemRandomizer gets rid of a compiler warning (unused var, seems to have been introduced here) so the output should be clean, and the post build update is a guard in case $(TargetDir) contains spaces.

I also realised with the SDK update recently that we switched to x64 builds so I think going forward we should release both as it's been x86 up until now.

One issue I think is that the packages will be double-zipped, but when releasing we can stick to manual uploads for now. The main purpose of this is for easier QA testing.
https://github.com/actions/upload-artifact#zipped-artifact-downloads

@lahm86 lahm86 requested review from chreden and makotocchi March 31, 2023 19:35
@lahm86 lahm86 force-pushed the issue-436-build-action branch from 7c5e23e to 19b54b1 Compare March 31, 2023 20:39
Copy link
Copy Markdown
Collaborator

@chreden chreden left a comment

Choose a reason for hiding this comment

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

Might be worth adding a dotnet test step? Not sure if there are any unit tests, but would mean you wouldn't have to change anything if any were added. I don't think it will fail if there are no tests but would be worth trying first.

@lahm86
Copy link
Copy Markdown
Collaborator Author

lahm86 commented Apr 1, 2023

Might be worth adding a dotnet test step? Not sure if there are any unit tests, but would mean you wouldn't have to change anything if any were added. I don't think it will fail if there are no tests but would be worth trying first.

Thanks, I did think about this too, but all the unit tests need local copies of the level files for 1-5. We don't keep these in the repository (and I don't think we should).
https://github.com/LostArtefacts/TR-Rando/tree/master/TRLevelReaderUnitTests

The tests do need expanding on so I agree it would be good to add this. I think we can exclude the ones above using this.

dotnet test --filter 'FullyQualifiedName!~TRLevelReaderUnitTests'

Ignores the actual tests for now as the level files are needed.
@lahm86 lahm86 merged commit eb92f8b into LostArtefacts:master Apr 1, 2023
@lahm86 lahm86 deleted the issue-436-build-action branch April 1, 2023 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants