-
Notifications
You must be signed in to change notification settings - Fork 153
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
Upgrade codebase to .NET6 and re-factor dependencies (1 of 3) #35
Upgrade codebase to .NET6 and re-factor dependencies (1 of 3) #35
Conversation
@Potherca just curious to know about any plans to review / merge this PR and the two that follow it. Are we looking for more reviewers? |
One reviewer is fine for now. At this point it is waiting for me or @RicardoNiepel to review the MRs. Sadly my planning is currently dominated by homeschooling my children, as we are stuck in self-quarantine because one of my daughters has COVID. I'm hoping to find some time during the week, but it could be at a standstill for another week. 😞 |
OK - no worries. Best wishes on your daughter's recovery! |
Hi @Potherca - Any plans to review these PRs? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally found some time to review this.
Everything looks alright. Some files are missing newline at end-of-file. I'll add those.
I'll run the code locally (I'm on Linux), after that, this can be merged.
Some observations of local run, @travisnielsen, could you take a look?:
|
Thanks for the review @Potherca ! I think I have addressed all the items from your testing:
|
I've updated my branch locally and run the code again.Everything seems to work just fine! 👍 There are some errors and warnings, but those are all application owned. All errors and warnings
|
@travisnielsen I am all set to merge this. Is there anything that needs to be done before I do so? After the merge #36 and #37 can be updated / rebased / rerun / etc. and then be merged as well. Thank you again for all your effort and patience to make this happen! |
Nope. We should be good to proceed with the merge!
Yep. I'll rebase the other two branches after the merge to main. Those should go much more quickly since they don't involve code changes. |
This PR replaces the C# Script (CSX) build process with a standard .NET6 project and removes dependencies on
rsvg
andinkscape
. PNG files are now generated using Playwright, which will result in much better consistency.