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

Improve source generator usage #301

Merged
merged 11 commits into from
Jun 22, 2021

Conversation

Turnerj
Copy link
Collaborator

@Turnerj Turnerj commented Jun 19, 2021

There are 3 main things this PR does:

  • The source generator now uses the latest previews of System.Text.Json
    This helps work around other weird Visual Studio behaviour when compiling the code and versions etc
  • Adds the flags so we can debug source generators via Visual Studio
    This is an amazingly useful feature!
  • Reverts to using a file for schema generation
    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.

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
@Turnerj Turnerj added enhancement Issues describing an enhancement or pull requests adding an enhancement. maintenance Pull requests that perform maintenance on the project but add no features or bug fixes. labels Jun 19, 2021
@Turnerj
Copy link
Collaborator Author

Turnerj commented Jun 19, 2021

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).

@RehanSaeed RehanSaeed added the patch Pull requests requiring a patch version update according to semantic versioning. label Jun 21, 2021
@RehanSaeed
Copy link
Owner

Perhaps we can add a CLI command to the tool to download the JSON-LD for us?

@Turnerj
Copy link
Collaborator Author

Turnerj commented Jun 21, 2021

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.

@Turnerj
Copy link
Collaborator Author

Turnerj commented Jun 21, 2021

So I've...

  • Moved the JSON-LD file to a new data folder in the root directory (seemed to make sense now we have two tools share it)
  • Added a basic .NET 5 CLI application that will download the latest file from Schema.org and save it to this folder

Seems to work pretty good.

@Turnerj
Copy link
Collaborator Author

Turnerj commented Jun 21, 2021

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.

@RehanSaeed
Copy link
Owner

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.

Copy link
Owner

@RehanSaeed RehanSaeed left a 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 👍🏼.

Tools/Schema.NET.Tool/Schema.NET.Tool.csproj Show resolved Hide resolved
Tools/Schema.NET.Updater/Program.cs Outdated Show resolved Hide resolved
@Turnerj
Copy link
Collaborator Author

Turnerj commented Jun 22, 2021

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.

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.

@Turnerj Turnerj merged commit 351ceb2 into RehanSaeed:main Jun 22, 2021
@Turnerj Turnerj deleted the improve-source-generator-usage branch June 22, 2021 13:05
@RehanSaeed
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues describing an enhancement or pull requests adding an enhancement. maintenance Pull requests that perform maintenance on the project but add no features or bug fixes. patch Pull requests requiring a patch version update according to semantic versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants