Skip to content

Conversation

@Juulsn
Copy link
Contributor

@Juulsn Juulsn commented Mar 13, 2023

Proposed change

Adds the JetBrains MeansImplicitUse to the NetDaemonAppAttribute.
Helps R# and Rider to mark classes as used if the NetDeamonAppAttribute is used

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]
  • The code compiles without warnings (code quality chek)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@FrankBakkerNl
Copy link
Contributor

FrankBakkerNl commented Mar 13, 2023

I like this idea, If I am correct there is a similar attributes in a MS namespace somewhere, that should probably work with both jet brains ans MS IDE and we would not have to reference JB packages

@helto4real
Copy link
Collaborator

Thank you for the contribution. I am reluctant to add this into the core since it is dependent on specific IDE:s. Is this possible to add as an Extension instead. I know the attribute class is sealed so I cannot see a good solution to it right now. @FrankBakkerNl ?

@Juulsn
Copy link
Contributor Author

Juulsn commented Mar 14, 2023

I've added the needed code directly as an internal class as stated on this site: https://blog.jetbrains.com/dotnet/2018/05/03/what-are-jetbrains-annotations/

So we can get rid of the nuget dependency.

@FrankBakkerNl I couldn't find anything similar in the MS namespace. JetBrains normally provides info about such attributes. For example the PureAttribute: https://www.jetbrains.com/help/resharper/Reference__Code_Annotation_Attributes.html#PureAttribute

@FrankBakkerNl
Copy link
Contributor

This is nice indeed, now Rider will no longer show my app classes in gray :-)
Because the jetbrains code is now included as internal, I don't see a problem like this, how about you @helto4real ?

@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.25 ⚠️

Comparison is base (254c154) 80.93% compared to head (342d07b) 80.68%.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #852      +/-   ##
==========================================
- Coverage   80.93%   80.68%   -0.25%     
==========================================
  Files         160      161       +1     
  Lines        4127     4137      +10     
  Branches      410      410              
==========================================
- Hits         3340     3338       -2     
- Misses        625      636      +11     
- Partials      162      163       +1     
Flag Coverage Δ
unittests 80.68% <0.00%> (-0.25%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...odel/Common/Attributes/JetBrainsCodeAnnotations.cs 0.00% <0.00%> (ø)
...ppModel/Common/Attributes/NetDaemonAppAttribute.cs 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@helto4real
Copy link
Collaborator

This is nice indeed, now Rider will no longer show my app classes in gray :-) Because the jetbrains code is now included as internal, I don't see a problem like this, how about you @helto4real ?

Yea this is much better solution. I accept that. Good solution. Thanks!

@helto4real helto4real merged commit 0d776c4 into net-daemon:dev Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants