-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: generic issues in EFragment #1117
fix: generic issues in EFragment #1117
Conversation
|
As always, sorry for being so slow. I wish I had much more time to contribute, because AA is really helps to have a cleaner code. |
|
Thanks, really appreciated! Can you look at your previous PR, i added a few |
|
This PR failed to build in Travis due to |
|
Yep, I've forgotten to run functional tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation. We use tabs.
|
I started to add indentation comments, but i realized you use spaces everywhere. Can you rebase? |
|
I could rebase, but not sure where? What commit should I use to rebase on? And how it helps? |
|
ServiceActionHandler has wrong indents. Should I reformat whole file? |
de0c1fe to
778485e
Compare
|
Looks like it is out of this PR |
|
Yeah, i will reformat that file later. Shoot, we have lots of formatting inconsistencies in this project, i should really create the checkstyle config. |
|
Yeah, it will help guys like me :) |
|
Checkstyle config will be my top priority after we release 3.1, i already started working on it, but it needs serious efforts. |
|
@yDelouis I reviewed and tested this PR, and i think this is ready to be merged. I just won't do it because i think it is too late to be in 3.1. |
|
Are you really sure, that it should be not in 3.1? Ofcource, it is possible, that I've broken something with PR, but without it we have broken functionality for sure. |
|
We want to release 3.1 today. Unfortunetaly it seems it will not happen, because @DayS is not responding, and currently he is the only one who has access rights to Maven Central etc. |
|
I see. If you are planning to release in a next few days, it is obviously to late to merge this commit. |
|
A release of that as a snapshot would be very nice... it is a current blocker on my current project... Looking forward to 3.1 anyway. |
|
If you use Maven, you can easily can build a snapshot from the source. But we are thinking about creating a 3.1 branch, so PRs can be merged the develop and snapshots will be available on OSS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tabs instead of spaces!
|
@Artyomcool, unfortunately there is a merge conflict. Also i have found a formatting problem. Can you rebase? |
|
@yDelouis i really want to merge this, because this is a serious defect, and effects multiple users. Unfortunately this has conflicts, so we could only merge this with conflict resolution merge commit. It would be much cleaner to rebase this and simply merge. |
|
Give an hour, I'll do the rebase |
|
Oh, great, thanks! BTW @yDelouis, my proposal still holds for other PRs. |
778485e to
60fd32d
Compare
|
done |
fix: generic issues in EFragment
|
@Artyomcool thanks! You should add the email you are using to your GitHub account, so it would be easier to see who created the commit. |
Fix for #1077.
It also could affect other generic issues in other enchanced classes because of using the actual class in BundleHelper.