Fixed Compiler errors, updated to newest java version, updated travis, updated to newest featureIDE #53
Conversation
This method has been deleted in FeatureIDE since this commit FeatureIDE/FeatureIDE@b679c39#diff-02b7a28d524fe3c717b03a9621030e760993f27c394403d3d837e36e4c4489fa Since there is no provision of a new method, it can be deleted. fixed tthuem#48
Deleted setPossibleWords(...)
accpted YaY
| } | ||
|
|
||
| @Override public boolean supportsPartialFeatureProject() { | ||
| return false; |
There was a problem hiding this comment.
Indent all over the place (tabsize = 8?)
There was a problem hiding this comment.
?
Edit by @pmbittner : Responding to this comment is impossible in Github. The same comment was made earlier here (#53 (comment)).
There was a problem hiding this comment.
this has already been fixed, but Github doesn't track the changes for some reason
de.tubs.variantsync.core/src/de/tubs/variantsync/core/patch/DeltaFactoryManager.java
Outdated
Show resolved
Hide resolved
|
@wurstbroteater @tcerny @jeremiaheinle Many of the requested changes have not been addressed, e.g. code style is still in an abysmal state, random lines are commented out, without any additional annotation. |
There was a problem hiding this comment.
Short: I don't see any progress.
Code Style: (reference: https://google.github.io/styleguide/javaguide.html)
- Indents are still quite often wild (please fix to tabsize = 4 spaces)
- All optional brackets must be given to avoid ambiguity ("dangling else")
- There is a plethora of undocumented magic strings and numbers, their purpose should be documented in form of a comment
-> If the style config is broken, then fix it.
General:
- Many methods are overridden but empty. Please document this stubbing in issues, so that it can be addressed in the future.
- Comment! Comment! Comment! Everything that is not self-explanatory should be commented, especially if it looks erroneous. There are over 40 stubbed methods, whose purpose is unclear.
| } | ||
|
|
||
| @Override public boolean supportsPartialFeatureProject() { | ||
| return false; |
There was a problem hiding this comment.
?
Edit by @pmbittner : Responding to this comment is impossible in Github. The same comment was made earlier here (#53 (comment)).
de.tubs.variantsync.core/src/de/tubs/variantsync/core/VariantSyncPlugin.java
Outdated
Show resolved
Hide resolved
de.tubs.variantsync.core/src/de/tubs/variantsync/core/VariantSyncPlugin.java
Outdated
Show resolved
Hide resolved
de.tubs.variantsync.core/src/de/tubs/variantsync/core/VariantSyncPlugin.java
Outdated
Show resolved
Hide resolved
| return "A " + (type == Type.CONFIGURATION ? "configuration project" : "variant") | ||
| + " does not exist in the workspace. " + getMessage(); |
There was a problem hiding this comment.
OK for now, please use StringBuilder in the future.
There was a problem hiding this comment.
This problem will be addressed by the following issue #56
de.tubs.variantsync.core/src/de/tubs/variantsync/core/utilities/FileHelper.java
Outdated
Show resolved
Hide resolved
de.tubs.variantsync.core/src/de/tubs/variantsync/core/utilities/FileHelper.java
Outdated
Show resolved
Hide resolved
de.tubs.variantsync.core/src/de/tubs/variantsync/core/utilities/Tree.java
Outdated
Show resolved
Hide resolved
de.tubs.variantsync.core/src/de/tubs/variantsync/core/view/context/ActiveContextHandler.java
Outdated
Show resolved
Hide resolved
| public void addHandlerListener(IHandlerListener handlerListener) { | ||
| } |
There was a problem hiding this comment.
I guess this is stubbing for futur work, please document in dedicated issue.
There was a problem hiding this comment.
This problem will be addressed by the following issue #57
|
Please ping me, when this PR is ready to be reviewed again. Please also add the newly generated issues to the PR commit message. |
There was a problem hiding this comment.
Finally, the golden sunshine of cleanly styled code has touched this battered code base. It is my delight, as his majesties reviewer, to approve this pull request and welcome it into its place in commit history.
Record screech
A few comments nevertheless:
- There are still commented out methods, else-if-cases and other stuff in the code base, please clean them up when you encounter them
- There are pros and cons for documenting the purpose of a method in Javadoc comments, please discuss with @pmbittner if this should be adopted from now on, to ease the learning curve for feature project participants and Paul himself.
fixed #45
fixed #46
fixed #47
fixed #48
fixed #42
During the review, it became apparent that there problems concerning the code style and documentation. This caused the new issues #56 and #57