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.

Sign up for GitHub

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

Jump to bottom

refactor: occurences -> occurrences #2206

Closed
wants to merge 2 commits into from
Closed

refactor: occurences -> occurrences #2206

wants to merge 2 commits into from

Conversation

zielint0
Copy link
Contributor

@zielint0 zielint0 commented Jul 11, 2018 •
edited
Loading

Fixed naming in names.

I am ready to merge.

@zielint0 zielint0 mentioned this pull request Jul 11, 2018
Open
@spoon-bot
Copy link
Collaborator

API changes: 5 (Detected by Revapi)

Old API: fr.inria.gforge.spoon:spoon-core:jar:7.1.0-20180711.225548-21 / New API: fr.inria.gforge.spoon:spoon-core:jar:7.1.0-SNAPSHOT

Field with constant value has been removed.
Old field ParameterInfo.UNLIMITED_OCCURENCES
New none
Breaking binary: non_breaking,
Method was removed.
Old method AbstractParameterInfo#getMaxOccurences()
New none
Breaking binary: breaking
Method was removed.
Old method AbstractParameterInfo#getMinOccurences()
New none
Breaking binary: breaking
Method was removed.
Old method AbstractParameterInfo#setMaxOccurences(int)
New none
Breaking binary: breaking
Method was removed.
Old method AbstractParameterInfo#setMinOccurences(int)
New none
Breaking binary: breaking

@surli
Copy link
Collaborator

surli commented Jul 12, 2018

The refactoring you propose here might break a lot of API and does not really bring a value for the developers, so I won't merge it.

@surli surli mentioned this pull request Jul 12, 2018
Closed
@monperrus
Copy link
Collaborator

this is a brand new, experimental API, so I would be OK to fix it right now. But I agree, in six months, it will be too late.

@pvojtechovsky ?

@pvojtechovsky
Copy link
Collaborator

pvojtechovsky commented Jul 12, 2018 •
edited
Loading

this is a brand new, experimental API, so I would be OK to fix it right now.

I agree with all of you Simon, Martin, Tomasz ;-)

I suggest to merge it now with the request to make new minor Spoon release 7.1 soon, then there will be not many clients influenced by this change. WDYT?

... it is interesting how much work might be to fix one missing r ... :-(

@zielint0
Copy link
Contributor Author

@pvojtechovsky

... it is interesting how much work might be to fix one missing r ... :-(

Och no, it was quick (Shift+F6).
than vs then was much longer!

@zielint0 zielint0 closed this Jul 12, 2018
@zielint0 zielint0 deleted the refactor-fix-naming-occurences branch July 12, 2018 20:05
@zielint0 zielint0 restored the refactor-fix-naming-occurences branch July 12, 2018 21:10
@zielint0 zielint0 reopened this Jul 12, 2018
@zielint0
Copy link
Contributor Author

zielint0 commented Jul 12, 2018 •
edited
Loading

@surli
@monperrus
@pvojtechovsky

By mistake I deleted branch.
I re-opened PR.
Sorry for confusion.
Previously all checks passed!

@surli
Copy link
Collaborator

surli commented Jul 13, 2018

could you create an empty commit to trigger back the CI please? And same for the other PRs

@zielint0
Copy link
Contributor Author

zielint0 commented Jul 13, 2018 •
edited
Loading

@surli
@monperrus

could you create an empty commit to trigger back the CI please? And same for the other PRs

All Travis green for:

@zielint0 zielint0 closed this Jul 14, 2018
@zielint0 zielint0 mentioned this pull request Jul 18, 2018
Merged
surli pushed a commit that referenced this pull request Jul 30, 2018
@zielint0 @surli
fix: typo in patterns (#2249)
f499611 
@monperrus 

This PR is follow up to refactor: fix naming occurences (re-introduced) #2214
As you merged #2214, this one should be too.
This PR finishes partial fix from #2214

Previous discussion: #2206
@pvojtechovsky 
> ... it is interesting how much work might be to fix one missing r ... :-(

It is even more - it looks like I forgot to rename:
- setMinOccurence method and its argument
- setMaxOccurence
- patterns

so I created this PR.

Because we cannot have pair:
- setMinOccurence (single *r*)
- setMaxOccurrence (double *r*)

this PR is required for consistency and to clean a mess. Sorry!

I think now we have consistent min/max methods & their arguments & correct patterns. Someone please take a look.

I am ready to merge.
spoon-bot added a commit to SpoonLabs/spoon-ci-data that referenced this pull request Jul 30, 2018
@spoon-bot
fix: typo in patterns (#2249)
af21d51 
@monperrus 

This PR is follow up to refactor: fix naming occurences (re-introduced) #2214
As you merged #2214, this one should be too.
This PR finishes partial fix from #2214

Previous discussion: INRIA/spoon#2206
@pvojtechovsky 
> ... it is interesting how much work might be to fix one missing r ... :-(

It is even more - it looks like I forgot to rename:
- setMinOccurence method and its argument
- setMaxOccurence
- patterns

so I created this PR.

Because we cannot have pair:
- setMinOccurence (single *r*)
- setMaxOccurrence (double *r*)

this PR is required for consistency and to clean a mess. Sorry!

I think now we have consistent min/max methods & their arguments & correct patterns. Someone please take a look.

I am ready to merge.
spoon-bot added a commit to SpoonLabs/spoon-ci-data that referenced this pull request Jul 30, 2018
@spoon-bot
fix: typo in patterns (#2249)
58bbc2c 
@monperrus 

This PR is follow up to refactor: fix naming occurences (re-introduced) #2214
As you merged #2214, this one should be too.
This PR finishes partial fix from #2214

Previous discussion: INRIA/spoon#2206
@pvojtechovsky 
> ... it is interesting how much work might be to fix one missing r ... :-(

It is even more - it looks like I forgot to rename:
- setMinOccurence method and its argument
- setMaxOccurence
- patterns

so I created this PR.

Because we cannot have pair:
- setMinOccurence (single *r*)
- setMaxOccurrence (double *r*)

this PR is required for consistency and to clean a mess. Sorry!

I think now we have consistent min/max methods & their arguments & correct patterns. Someone please take a look.

I am ready to merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zielint0 @spoon-bot @surli @monperrus @pvojtechovsky