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

fix: correct spelling mistake in API, doc and tests (#5934) #5943

Conversation

PinkMoustacheMan
Copy link
Contributor

  • mark misspelled method as deprecated
  • add new method with correct spelling
  • correct spelling of method in docs and test classes

- mark misspelled method as deprecated
- add new method with correct spelling
- correct spelling of method in docs and test classes
@monperrus
Copy link
Collaborator

thanks for the PR. would you have a look at the failing CI job?

Copy link
Collaborator

@SirYwell SirYwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the method added to the interface be default and delegate to the deprecated method to avoid breaking changes?

@PinkMoustacheMan
Copy link
Contributor Author

Should the method added to the interface be default and delegate to the deprecated method to avoid breaking changes?

I believe it should be the other way around.
The newly added method is the "correct" one. If its implementation changes (let's just assume the changing implementation is backwards compatible) then the deprecated method should follow suit.
The "incorrect" method should not be the one that is actively being maintained.

@monperrus
Copy link
Collaborator

Per the theory of software engineering, there should be no delegation:

  • the deprecated method is frozen, kept as is for backward compatibility, and will be removed in the future after the deprecation period (do we have an official one in Spoon?)
  • the new method is the one actively maintained.

Here, the code is so simple, so I'm happy to merge anyway when CI is green.

- add missing javadoc properties
- change indentation from spaces to tabs
@PinkMoustacheMan PinkMoustacheMan force-pushed the fix-spelling-mistake-in-annotation-processor-api branch from 7ea0ad7 to b49327c Compare August 26, 2024 17:01
@PinkMoustacheMan
Copy link
Contributor Author

I do agree with the notion that deprecated methods should not change anymore, so I reverted that change.
I also fixed the whitespace issue, I thought this was not part of the checks (or that it would be fixed as part of a commit hook or CI step).

@monperrus
Copy link
Collaborator

Thanks.

Why do we have a public and a private version of the same method AFAIU? see private boolean shouldBeProcessed

@PinkMoustacheMan
Copy link
Contributor Author

There is a public shouldBeConsumed method as well as a private shouldBeProcessed method.
They are doing the same thing but referencing different fields.

@monperrus monperrus merged commit 0bcc1a5 into INRIA:master Aug 28, 2024
12 checks passed
@monperrus
Copy link
Collaborator

thanks a lot @PinkMoustacheMan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants