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

chore: rename tests add derived interface test #1816

Merged
merged 3 commits into from
Sep 17, 2024

Conversation

TimothyMakkison
Copy link
Contributor

  • Renamed DerivedRefitInterfaceTest and DerivedNonRefitInterfaceTest
  • Added InterfaceDerivedFromRefitBaseTest

@TimothyMakkison
Copy link
Contributor Author

@ChrisPulman did you have any thoughts regarding my comment? The lack of incremental generator tests are the main blocker preventing me from adding incremental generation.

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.74%. Comparing base (6ebeda5) to head (8f41deb).
Report is 91 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1816      +/-   ##
==========================================
- Coverage   87.73%   83.74%   -3.99%     
==========================================
  Files          33       36       +3     
  Lines        2348     2449     +101     
  Branches      294      345      +51     
==========================================
- Hits         2060     2051       -9     
- Misses        208      316     +108     
- Partials       80       82       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ChrisPulman
Copy link
Member

@ChrisPulman did you have any thoughts regarding my comment? The lack of incremental generator tests are the main blocker preventing me from adding incremental generation.

Could we go with Add a new InterfaceStubGenerator.Roslyn41 option for the time being, this way we can maintain older versions as they currently exist and open a path to pure incremental generators, we can drop the relevant support if required once we get a stable version of 41.

@ChrisPulman
Copy link
Member

Roslyn 38, 40 and 41 are all supported by Rider, VS2019 (Fully updated) and VS2022, so really using Roslyn 41 shouldn't be an issue.

@ChrisPulman
Copy link
Member

I am happy with updating the version of Rosyln to a single newer version rather than trying to support 2 or 3 versions, this would involve changing the project setup to have a single NetStandard project instead of a shared project.
Would you be okay with going down that route?
While understanding that we will drop support for some versions of Visual Studio we can advance Refit to be what people are calling for. So, while keeping a version as low as possible i.e. Roslyn 41, I am happy with anything up to Roslyn 46 if that makes anything easier.

@ChrisPulman ChrisPulman merged commit d63d98d into reactiveui:main Sep 17, 2024
2 of 3 checks passed
@TimothyMakkison TimothyMakkison deleted the derive_test branch September 17, 2024 14:55
@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Sep 17, 2024

Would you be okay with going down that route?

I'm happy with either route. My current solution was to update Roslyn 4.0 to Roslyn 4.1, this way we keep support for 3.9.

we will drop support for some versions of Visual Studio we can advance Refit to be what people are calling for.

Personally, none of my planned changes require dropping support for anything. I only need 4.1 to use WithTrackingName to determine if the generator is caching correctly. This would be an extremely minimal change regardless of which approach is taken.

I am happy with anything up to Roslyn 46 if that makes anything easier.

This should be a separate issue but I did wonder if ForAttributeWithMetadataName could be used to look for interfaces containing the Refit Http attributes.

Copy link

github-actions bot commented Oct 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants