Skip to content

Conversation

@solomax
Copy link
Contributor

@solomax solomax commented Sep 20, 2018

Additionally I would like to remove
ReferenceMap.HARD, SOFT, WEAK in favor of ReferenceStrength.HARD, SOFT, WEAK

shall I add another commit to this PR, or create another one?

this fix depends on commons-collections4 4.3-SNAPSHOT, will compile as soon as 4.3 will be released or SNAPSHOT will be uploaded

@ilgrosso
Copy link
Member

Thx @solomax - this PR looks good to me.

I would like to remove ReferenceMap.HARD, SOFT, WEAK in favor of ReferenceStrength.HARD, SOFT, WEAK
shall I add another commit to this PR, or create another one?

To me, you can add that here.

@struberg / @rmannibucau would you please doublecheck?

Copy link
Contributor

@rmannibucau rmannibucau left a comment

Choose a reason for hiding this comment

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

Looks ok to me (even adding a commit)
Side note: can be good to remove style/@OverRide changes since they are unrelated and take most of the diff

@solomax
Copy link
Contributor Author

solomax commented Sep 20, 2018

@rmannibucau I have this setting on by default in my IDE due to I had extremely bad experience of project migration without @OverRide being set.
Shall I remove all these annotations?

@rmannibucau
Copy link
Contributor

@solomax if possible it would be awesome to have this PR about commons-collection upgrade only and then if you want add override everywhere

@solomax
Copy link
Contributor Author

solomax commented Sep 20, 2018

sure, will do
will take some time :)

@solomax
Copy link
Contributor Author

solomax commented Sep 20, 2018

Hopefully this diff will be easier to read :)

@ilgrosso
Copy link
Member

Ok, I am going to merge this PR right now - but please don't forget to submit another for @overrides ;-)

@solomax
Copy link
Contributor Author

solomax commented Sep 20, 2018

@ilgrosso this will break the build ... most probably
4.3-SNAPSHOT might not be online ...

@ilgrosso
Copy link
Member

@solomax let me check..

@ilgrosso
Copy link
Member

@solomax ah I see: do you know why 4.3-SNAPSHOT is not there yet?

@solomax
Copy link
Contributor Author

solomax commented Sep 20, 2018

I'm afraid there no auto build :(
I've asked the devs: https://markmail.org/thread/tshcd4d54nxrvgbu

Will send additional email regarding SNAPSHOT

@ilgrosso
Copy link
Member

@solomax could you please rebase?
Anyway, I cannot see yet commons-collections4's 4.3-SNAPSHOT artifacts out there.. :-(

@solomax
Copy link
Contributor Author

solomax commented Sep 24, 2018

@ilgrosso could you please guide me with rebase?
I never used it :(

@solomax
Copy link
Contributor Author

solomax commented Sep 24, 2018

I wrote another email regarding commons-collections4's 4.3-SNAPSHOT
no answer yet :(

@ilgrosso
Copy link
Member

could you please guide me with rebase?
I never used it :(

I mean something like as https://robots.thoughtbot.com/keeping-a-github-fork-updated

@solomax solomax force-pushed the commons-collections branch from 617ebf5 to 58a397a Compare September 24, 2018 12:43
@solomax
Copy link
Contributor Author

solomax commented Sep 24, 2018

Just have tried to rebase but no luck :(
rebase against solomax/trunk has no effect
rebase against apache/trunk conflicts and I have no idea how to resolve it :(

@ilgrosso
Copy link
Member

rebase against apache/trunk conflicts and I have no idea how to resolve it :(

yep, you should rebasing against apache/trunk and solving the conflicts :-)
There have been no changes in trunk but yours, so I think you should be able to cope with those...

@solomax solomax force-pushed the commons-collections branch from 58a397a to 30eaad0 Compare September 24, 2018 12:52
@solomax
Copy link
Contributor Author

solomax commented Sep 24, 2018

OK, the trick was to use git rebase --skip :)
But for some reason there are still 3 commits :(

@ilgrosso
Copy link
Member

But for some reason there are still 3 commits :(

This could be fixed too, but it is not really a big issue since we are with SVN.

@solomax
Copy link
Contributor Author

solomax commented Sep 24, 2018

Great :)

@ilgrosso
Copy link
Member

ah, it seems you can merge this PR by yourself, when the commons-collection4 artifacts will be available ;-)
congrats!

@solomax
Copy link
Contributor Author

solomax commented Sep 24, 2018

Sure, will do :)
But I would like to perform contributions via PRs (at least for the first time)
Would it be OK?

@ilgrosso
Copy link
Member

But I would like to perform contributions via PRs (at least for the first time)
Would it be OK?

I don't see any issue with that.

@solomax
Copy link
Contributor Author

solomax commented Sep 24, 2018

Great, so will do the merge as soon as SNAPSHOT will be available
Thanks for the help!

@solomax
Copy link
Contributor Author

solomax commented Sep 25, 2018

Merged with 4709f38

@solomax solomax closed this Sep 25, 2018
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