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.
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
Add concrete JsPlugin #4925
Add concrete JsPlugin #4925
Changes from 11 commits
7808908
e3ccbc1
2fed742
bd6f098
947a5ec
24c6063
bcde5ff
3eb9c3d
5132e10
c23950f
0522799
60731f1
eece802
d8fe591
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Should note that
name
can have a/
in it for subfolders. Likely important to note that we do not want to URL encode that/
, we want it to actually be atjs-plugins/@scope/name
, notjs-plugins/@scope%2Fname
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.
#4950
This file was deleted.
This file was deleted.
This file was deleted.
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.
We might consider a different class name here - when dealing with
java.nio.file.Path
instances, it is common to usejava.nio.file.Paths
to create them.Also, javadoc
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.
I'm not sure how to best rename it if necessary - I'm personally less offended by names that overlap in these regards, although I know others disagree. At least
java.nio.file.Paths
is somewhat discouraged in their javadoc: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.
Unsolicited advice, and unimportant for this iteration, but might be handy if considering using PathMatcher more generally:
FileSystems.getDefault().getPathMatcher(...)
supportsglob:
patterns, but if you do, take care to note that**/*
does not match files in the current dir (that is, something which*
would match).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.
Yeah - I considered making PathMatcher the public API, but I don't know what sort of interface limitations we might wish we had imposed if we want an "easy" route-based integration w/ jetty.