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

Revisit all TODO and remove them #303

Closed
adrianoc opened this issue Sep 11, 2024 · 0 comments
Closed

Revisit all TODO and remove them #303

adrianoc opened this issue Sep 11, 2024 · 0 comments
Milestone

Comments

@adrianoc
Copy link
Owner

Cases that are still relevant should have an issue created

@adrianoc adrianoc added this to the 2.15 milestone Sep 11, 2024
@adrianoc adrianoc changed the title Revisit all TODOs and either remove it Revisit all TODOs and remove it Sep 11, 2024
@adrianoc adrianoc changed the title Revisit all TODOs and remove it Revisit all TODO s and remove it Sep 11, 2024
@adrianoc adrianoc changed the title Revisit all TODO s and remove it Revisit all TODO and remove them Sep 11, 2024
@adrianoc adrianoc modified the milestones: 2.15, 2.16 Sep 29, 2024
adrianoc added a commit that referenced this issue Oct 12, 2024
This TODO mentions checking missing types but the code seems to be handling this scenario
in [line 107](https://github.com/adrianoc/cecilifier/blob/main/Cecilifier.Core.Tests/Framework/AssemblyDiff/AssemblyComparer.cs#L107)
adrianoc added a commit that referenced this issue Oct 13, 2024
adrianoc added a commit that referenced this issue Oct 13, 2024
In this case I think introducing a constant would only make the code less readable
adrianoc added a commit that referenced this issue Oct 13, 2024
#303)

the comment mentions abstract static methods from interfaces but the method in question should only
be used while emitting synthetic methods (for instance, Add/Remove for events, get_/set_/etc for
properties, etc) and in these scenarios we probably don´t support abstract static methods.
adrianoc added a commit that referenced this issue Oct 13, 2024
the comment claims that Equals(RecordType) overload should not have 'MethodAttributes.NewSlot' applied but
it seems it is not the case (the compiler does add this attribute)
adrianoc added a commit that referenced this issue Oct 13, 2024
adrianoc added a commit that referenced this issue Oct 18, 2024
Also uses right parameter attributes instead of hardcoded None
adrianoc added a commit that referenced this issue Oct 18, 2024
adrianoc added a commit that referenced this issue Oct 18, 2024
adrianoc added a commit that referenced this issue Oct 18, 2024
TODO was questioning whether it would make sense to register a variable definition; it may or may not
improve the generated code quality but the method in question is only being used to handle events
add/remove methods so it does not worth investigating it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant