10000 chore: Fix and Reconfigure GitHub Actions by mikechu-optimizely · Pull Request #319 · optimizely/csharp-sdk · GitHub
[go: up one dir, main page]
Skip to content

chore: Fix and Reconfigure GitHub Actions #319

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

Merged
merged 5 commits into from
Nov 15, 2022
Merged

chore: Fix and Reconfigure GitHub Actions #319

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
35d7921
Edits to GitHub Actions
mikechu-optimizely Nov 14, 2022
05c59f9
Changes to .editorconfig & App.config for test project
mikechu-optimizely Nov 14, 2022
c78e858
PR review changes + extra doc cleanup
mikechu-optimizely Nov 15, 2022
c5e07ac
Remove quotations from env var
mikechu-optimizely Nov 15, 2022
7f30fe5
Return to using Travis solution...
mikechu-optimizely Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Return to using Travis solution... 
though it should be rename maybe.
  • Loading branch information
@mikechu-optimizely
mikechu-optimizely committed Nov 15, 2022
commit 7f30fe5eca3cef94a66af1126fcc9d7c5c5ebcf0
2 changes: 1 addition & 1 deletion .github/workflows/csharp.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jobs:
- name: Restore NuGet packages
run: nuget restore ./OptimizelySDK.Travis.sln
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please run OptimizelySDK.sln instead of OptimizelySDK.Travis.sln

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The OptimizelySDK.Travis.sln does not include the demo app OptimizelySDK.DemoApp.csproj which has build errors.

I'm guessing the demo app was not needed, so someone created the Travis version.

In later sprints, I think we should fix or remove the demo app.

It's likely safe to continue to build the Travis solution instead.

- name: Build solution
run: msbuild /p:SignAssembly=true /p:AssemblyOriginatorKeyFile=$(pwd)/keypair.snk /p:Configuration=Release ./OptimizelySDK.sln
run: msbuild /p:SignAssembly=true /p:AssemblyOriginatorKeyFile=$(pwd)/keypair.snk /p:Configuration=Release ./OptimizelySDK.Travis.sln
- name: Install NUnit Console
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move it before running msbuild. SO all setups can be seen together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. That makes sense.

I intended to only execute installations before they were needed in case of failure. eg If build failed, no need to install NUnit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 Thinking more on this… I'd like to think green and code sustainably here.

By performing the installation only if and just before it's needed, we're reducing the amount of resources (albeit small) being used on a potentially unnecessary step.

If you feel strongly over this re-ordering, please DM me on Teams, and I'll roll it into another PR.

run: nuget install NUnit.Console -Version 3.15.2 -DirectDownload -OutputDirectory .
- name: Run NUnit tests
Expand Down
0