-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Updating various old/moved URL references found across project (jetty-10.0.x
)
#10098
Conversation
…lipse.dev/jetty/` has occurred, it is time to review the URI use in our project + Updated URLs in poms + Added more URIs to XmlConfiguration + Updated URLs in module files + Updated URLs in documentation + Updated URLs in HTML + Correcting bad double-scheme URLs (eg: `http://https://www.eclipse...`) Signed-off-by: Joakim Erdfelt <[email protected]>
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.
mostly good, just a small change to language needed.
@@ -1,4 +1,4 @@ | |||
# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html | |||
# DO NOT EDIT THIS FILE - Use start modules correctly - See: https://eclipse.dev/jetty/documentation/ |
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 think the "Use start modules correctly" is a bit scolding. Let's keep it factual:
# DO NOT EDIT THIS FILE - Use start modules correctly - See: https://eclipse.dev/jetty/documentation/ | |
# DO NOT EDIT THIS FILE - See: https://eclipse.dev/jetty/documentation/ |
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 removed the reference to /current/startup-modules.html
as that is no longer a valid URL.
Pointing to the documentation alone is insufficient. (what do they look for?)
Perhaps it should read ...
# DO NOT EDIT THIS FILE - See startup modules documentation at https://eclipse.dev/jetty/documentation/
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.
Or ...
# DO NOT EDIT THIS FILE - See Jetty Start Mechanism in Operations Guide at https://eclipse.dev/jetty/documentation/
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.
Or ...
# DO NOT EDIT THIS FILE
# See https://eclipse.dev/jetty/documentation/ > Operations Guide > Jetty Start Mechanism
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.
'/current' under the documentation is really old and not a maintained documentation URL atm (last updated 3 yrs ago)
Look here for current exposed layout.
That is how things are currently structured, talking with @sbordet and @lorban yesterday we agreed we should remove the content under '/current' and just provide a redirect to '/documentation'
Signed-off-by: Joakim Erdfelt <[email protected]>
This reverts commit 4a02e22.
Signed-off-by: Joakim Erdfelt <[email protected]>
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.
Fine to commit, though I do prefer the more descriptive language in an earlier suggestion of yours. Gives people some direction on where to go regardless of major Jetty version.
See https://eclipse.dev/jetty/documentation/ > Operations Guide > Jetty Start Mechanism
@@ -1896,7 +1896,7 @@ else if (arg.toLowerCase(Locale.ENGLISH).endsWith(".properties")) | |||
} | |||
} | |||
|
|||
private static class ConfigurationParser extends XmlParser implements AutoCloseable | |||
protected static class ConfigurationParser extends XmlParser implements AutoCloseable |
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 think this could go back to private
, and modify getParser()
to return XmlParser
which is a public class. See also comment in XmlConfigurationTest
.
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.
Done.
Since ConfigurationParser
was closeable, and XmlParser
isn't, I had to juggle that requirement too.
{ | ||
// empty raw xml, just to instantiate XmlConfiguration, so we can access the ConfigurationParser. | ||
XmlConfiguration xmlConfiguration = asXmlConfiguration("<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\" />"); | ||
XmlConfiguration.ConfigurationParser configurationProcessor = xmlConfiguration.getParser(); |
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 would rename getParser()
to getXmlParser()
and return type to be XmlParser
, which is already a public class.
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.
Done
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
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.
Found a few more things to correct
documentation/jetty-documentation/src/main/asciidoc/old_docs/runner/jetty-runner.adoc
Outdated
Show resolved
Hide resolved
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
@@ -72,7 +72,7 @@ | |||
<version>${project.version}</version> | |||
<prog-guide>../programming-guide/index.html</prog-guide> | |||
<op-guide>../operations-guide/index.html</op-guide> | |||
<javadoc-url>https://www.eclipse.org/jetty/javadoc/jetty-10</javadoc-url> | |||
<javadoc-url>https://eclipse.dev/jetty/javadoc/jetty-12</javadoc-url> |
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 be /jetty-10
at the end (and changed during merging forward).
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 caught the other one I changed too early, missed this one.
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.
Done.
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Notes for merging: These version references need to be updated in each merge. |
Signed-off-by: Joakim Erdfelt <[email protected]>
| https://github.com/eclipse/jetty.project/tree/jetty-9.4.x[jetty-9.4.x] | End of Community Support | Servlet 3.1 | Java 8 | ||
| https://github.com/eclipse/jetty.project/tree/jetty-9.3.x[jetty-9.3.x] | End of Life | Servlet 3.1 | Java 8 | ||
| jetty-8 | Historical | Servlet 3.1 | Java 6 | ||
| jetty-7 | Mythical | Servlet 2.5 | Java 5 |
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.
The title of this table is "Active Eclipse Jetty Branches".
I don't think jetty-8 and jetty-7 make the grade for this table.
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.
And even jetty-9.3.x
isn't considered "Active" anymore.
Signed-off-by: Joakim Erdfelt <[email protected]>
Stale review - requested changes have been applied, this is holding up the ability to merge.
…w-eclipse.dev-uris
* Updating various old/moved URL references found across project (`jetty-10.0.x`) (#10098) + Now that the migration of `https://eclipse.org/jetty/` to `https://eclipse.dev/jetty/` has occurred, it is time to review the URI use in our project + Updated URLs in poms + Added more URIs to XmlConfiguration + Updated URLs in module files + Updated URLs in documentation + Updated URLs in HTML + Correcting bad double-scheme URLs (eg: `http://https://www.eclipse...`) + Updating text in *.mod files + Removing `/current/` from path `/jetty/documentation/current/` + Fixing mailing list URL + Fixing github URL references in jsps --------- Signed-off-by: Joakim Erdfelt <[email protected]>
…y-10.0.x`) (#10098) * Now that the migration of `https://eclipse.org/jetty/` to `https://eclipse.dev/jetty/` has occurred, it is time to review the URI use in our project + Added more URIs to XmlConfiguration --------- Signed-off-by: Joakim Erdfelt <[email protected]>
…er` in `XmlParser` (#10299) Backports of * Issue #10066 - Allow customization of `SAXParserFactory` and `SAXParser` in `XmlParser` (#10067) * Updating various old/moved URL references found across project (`jetty-10.0.x`) (#10098) Consisting of * Allow customization of SAXParserFactory / SAXParser in XmlParser * Introduce method `.getSAXParser()` * Prepending doctype in testcases * More comprehensive changes to redirectEntity config * Updating various old/moved URL references found across project (`jetty-10.0.x`) (#10098) * Now that the migration of `https://eclipse.org/jetty/` to `https://eclipse.dev/jetty/` has occurred, it is time to review the URI use in our project * Added more URIs to XmlConfiguration * Better SAXParseException handling to report resource that is causing the problem * Add missing DOCTYPE declarations * Enforcing unique XML ids --------- Signed-off-by: Joakim Erdfelt <[email protected]> Co-authored-by: Greg Wilkins <[email protected]>
Now that the migration of
https://eclipse.org/jetty/
tohttps://eclipse.dev/jetty/
has occurred, it is time to review the URI use in our projecthttp://https://www.eclipse...
)