refactor: Big reorg for handling sources #1272
Merged
Conversation
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
Seems that PowerShell is still bound by some pretty limmited command lines buffers. So we'll just apply the same rules for PowerShell as we do for CMD. Fixes jbangdev#1253
The code for (recursively) obtaining all script information was all centered on `ScriptSource` which made it hard to create/add alternative sources of information. This is the first step on the way to remedy that.
No code was added in this refactor but the hierarchy was changed extensively. First what was before called `Source` is now called `Input` with the only implementing classes `Jar` and `SourceSet`. The `ScriptSource` and child classes are now simply named `Script` and don't inherit from `Source` anymore. And Java isn't the default anymore for `Script`, but a separate `JavaScript` (yes yes) was created and `Script` is now abstract.
maxandersen
reviewed
Mar 2, 2022
maxandersen
reviewed
Mar 2, 2022
maxandersen
reviewed
Mar 3, 2022
quintesse
force-pushed
the
script_refactor3
branch
from
a62fa51 to
b0695d4
Compare
maxandersen
requested changes
Mar 3, 2022
Instead of a single `Builder` implementation (`JarBuilder`) we now use a base implementation and several implementations, one for each of the support programming languages. `SourceSet`s now know how they should be built and can return the correct `Builder` for their sources. A bunch of code was moved from the different `Script` classes to the new `Builder` classes.
Now that `Source` wasn't used anymore we could use it again for the classes that represent source files.
`Input` wasn't really a good name because when a `SourceSet` gets built it's turned into a `Jar` which is an ouput, not an input. While both a `SourceSet` and a `Jar` represent code, so `Code` seems like a good generic name.
The `DefaultCmdGenerator` was split into a `BaseCmdGenerator` and pair of implementations for running jars and jshell scripts.
quintesse
force-pushed
the
script_refactor3
branch
from
b0695d4 to
3fa419d
Compare
quintesse
force-pushed
the
script_refactor3
branch
from
3de0530 to
d8aa209
Compare
Codecov Report
@@ Coverage Diff @@
## main #1272 +/- ##
============================================
+ Coverage 58.26% 58.76% +0.49%
- Complexity 1077 1126 +49
============================================
Files 86 96 +10
Lines 5820 5963 +143
Branches 997 991 -6
============================================
+ Hits 3391 3504 +113
- Misses 1927 1955 +28
- Partials 502 504 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Tests have passed yay |
| import dev.jbang.source.RunContext; | ||
| import dev.jbang.source.ScriptSource; | ||
| import dev.jbang.source.Source; | ||
| import dev.jbang.source.*; |
There was a problem hiding this comment.
Are * based imports fine?
There was a problem hiding this comment.
we don't have a strong rule against it - but prefer to keep it to minimum.
There was a problem hiding this comment.
Honestly I thought Spotless automatically removed all of them.
|
some of the naming i'm not a super fan of but I prefer to live with it until we find better names (if ever :) than not get this refactor in. this will make great things easier! thanks @quintesse ! |
This was referenced Mar 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Lot's of changes here, but basically it boils down to 3 things:
SourceSet. It contains all the information about a source project; sources, resources, dependencies etc etc. but without defining how that information is obtained. This will allow us in the future to define alternative project types. The new builders use this and are already much more agnostic about JBang-specific scripts. This is a big step but not completely done yet, there are still quite a number of places where assumptions are being made about the project structure but we can weed those out bit by bit in the coming time.