-
Notifications
You must be signed in to change notification settings - Fork 121
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
2390 #3090
Open
l3r8yJ
wants to merge
4
commits into
objectionary:master
Choose a base branch
from
l3r8yJ:2390
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
2390 #3090
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -246,7 +246,7 @@ private List<Path> transpile( | |
|
||
/** | ||
* Clean up dirty classes. | ||
* The method is trying to fix problem produced by dirty libraries: | ||
* The method is trying to fix a problem produced by dirty libraries: | ||
* <a href="https://github.com/objectionary/eo-strings/issues/147"> eo-strings example </a> | ||
* Some libraries by mistake can put ALL their compiled classes right into the final library | ||
* jar, instead of only adding atoms. This can cause different runtime errors since the | ||
|
@@ -265,12 +265,15 @@ private List<Path> transpile( | |
* {@link java.nio.file.AccessDeniedException}, which could crash the build. | ||
* _____ | ||
* @param java The list of java files. | ||
* @todo #2375:90min. Add concurrency tests for the TranspileMojo.cleanUpClasses method. | ||
* We should be sure that the method works correctly in a concurrent environment. | ||
* In order to do so we should add a test that will run the cleanUpClasses method in | ||
* multiple threads and check that the method works correctly without exceptions. | ||
* We can apply the same approach as mentioned in that post: | ||
* <a href="https://www.yegor256.com/2018/03/27/how-to-test-thread-safety.html">Post</a> | ||
* @todo #2375:90min. Implement mechanism for "inner" and "outer" classes. | ||
* To get rid of TranspileMojo#cleanUpClasses, we can implement mechanism, | ||
* that will mark classes of the project like "inner", which is will be checked | ||
* during the compilation process. | ||
* Another solution is to create analog of gradle `implementation` and `api`, | ||
* - implementation – just includes the dependency for inner usage | ||
* - api – allows to users of API to | ||
* use dependency which was added to project via `api` keyword | ||
* <p/><a href="https://shorturl.at/abns4">More about api and implementation here</a> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @l3r8yJ Does this link actually lead somewhere? I can't open it. |
||
*/ | ||
private void cleanUpClasses(final Collection<? extends Path> java) { | ||
final Set<Path> unexpected = java.stream() | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@l3r8yJ To be honest, I didn't clearly understand what you want to do in this puzzle.
First of all. Do you want to "get rid of the TranspileMojo#cleanUpClasses method?" If it is so, it would be nice to put this phrase as a header of the puzzle:
Second, it would be nice to answer the "why?" question. Why do we need to do it at all?
Third, you can suggest some ideas about the possible solutions of the problem you already mentioned.
However, these are only my suggestions, and you can do it as you want. But please, make this puzzle a bit more understandable.
BTW, I didn't clearly understand what "inner" and "outer" classes mean.