-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Improve source generator usage #301
Improve source generator usage #301
Conversation
While the source generator doesn't use any new function of the S.T.J preview, it does help workaround issues with compiling the project in Visual Studio. There is something weird with the versions, transient dependencies and some incompatibility even when specifying all the dependencies properly.
Allows you to debug the source generator like it was any other code
So now I've gone from "it builds in CI but doesn't build locally" to "it builds locally but doesn't build in CI"... 🤦♂️ Well more specifically: it builds via Visual Studio/MSBuild (.NET Framework) now but doesn't via dotnet CLI (.NET Core). |
They aren't technically required on the CI but makes the problem more obvious
Perhaps we can add a CLI command to the tool to download the JSON-LD for us? |
It would have to be an additional tool as we can't call the source generator one from CLI but yeah, we could have a thin schema download client. I still wouldn't want to call it in our own build process but it does simplify our updating process. I'll mock something up as another commit to this PR. |
So I've...
Seems to work pretty good. |
I'm thinking in the future, using something like this with a scheduled GitHub Action to do this updating for us. That though is a whole separate thing we can handle another day. |
We have to be careful with code that updates itself. We need to ensure there are no breaking changes, these normally fail the build/tests though. A better approach might be to have a GitHub action that raises a PR for us a bit like dependabot/renovate. |
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.
Seems reasonable to me 👍🏼.
That's a good idea. I won't look too much into it yet but I think if we had something open a PR every time there was a new schema.org version, that would be useful. |
It'd also then play nicely with the auto-generated release notes. There will be a manual step of clicking the publish button every so often though. |
There are 3 main things this PR does:
This helps work around other weird Visual Studio behaviour when compiling the code and versions etc
This is an amazingly useful feature!
Something wasn't sitting right with me that our builds were not deterministic - it was up to the state of the JSON file downloaded. I thought via a file might be faster too but so far haven't seen that be the case.
With that last point, I think we can still semi-automate the updating process for the JSON file I've embedded via scheduled GitHub Actions. This might also allow us to support #165 more easily.
In theory, none of these changes should affect the output of Schema.NET, just the build/debugging process.