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

Add Pull Request Github workflow #96

Merged
merged 12 commits into from
Dec 3, 2021
Merged

Add Pull Request Github workflow #96

merged 12 commits into from
Dec 3, 2021

Conversation

Skrypt
Copy link
Contributor

@Skrypt Skrypt commented Nov 27, 2021

Fixes #95, Fixes #80

See :

image

@Skrypt
Copy link
Contributor Author

Skrypt commented Nov 27, 2021

Failing because of net6.0

@Skrypt
Copy link
Contributor Author

Skrypt commented Nov 27, 2021

We would need to multi-target like we do in OC. @bleroy does that make sense or you want to only keep one framework?

@bleroy
Copy link
Member

bleroy commented Nov 27, 2021

It makes sense, but if that's too much work, latest supported by OC wouldn't be terrible

@Skrypt
Copy link
Contributor Author

Skrypt commented Nov 27, 2021

I could add a common .csproj as we do in OC to add vars for these that we could reuse in each project. Not too much work, I just copy some stuff I already have and adapt it.

@Skrypt
Copy link
Contributor Author

Skrypt commented Nov 27, 2021

I will make a different PR for multi-targeting. That's mainly why I had copied the main OC repository structure. To be able to copy/paste these files without having to adapt them for both projects.

@Skrypt Skrypt closed this Dec 3, 2021
@Skrypt Skrypt reopened this Dec 3, 2021
@Skrypt
Copy link
Contributor Author

Skrypt commented Dec 3, 2021

Finally building and passing unit tests!

The issue was that when we are specifying a unique framework in the projects then we should not pass the --framework argument on the command line when doing dotnet build.

In the future though, when we move to multi-targeting it will make sense to pass the --framework argument.

So here the CI works with what we have before moving to .NET 6.0.

@Skrypt
Copy link
Contributor Author

Skrypt commented Dec 3, 2021

Ok I'm done adding commits if you want to merge @bleroy

@bleroy bleroy merged commit 6d43af0 into master Dec 3, 2021
@bleroy
Copy link
Member

bleroy commented Dec 3, 2021

Thanks for sorting this out!

@bleroy bleroy deleted the skrypt/#95 branch December 3, 2021 23:20
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.

Add Github Actions for pull requests Culture is not supported
2 participants