-
Notifications
You must be signed in to change notification settings - Fork 31
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
MTA 7.0: Rules development guide -- Broken link fix #893
base: main
Are you sure you want to change the base?
Conversation
d63297c
to
d537814
Compare
docs/topics/testing-rules.adoc
Outdated
@@ -99,7 +99,7 @@ The `<not>` element has no unique attributes or child elements. | |||
[discrete] | |||
===== Summary | |||
|
|||
The `<iterable-filter>` element counts the number of times a condition is verified. For additional information, see the link:{LinkAPI}org/jboss/windup/rules/general/IterableFilter.html[IterableFilter] class. | |||
The `<iterable-filter>` element counts the number of times a condition is verified. For additional information, see the link:https://github.com/windup/windup/blob/master/rules-base/api/src/main/java/org/jboss/windup/rules/general/IterableFilter.java[IterableFilter] 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.
Why have you not used the LinkAPI
attribute for consistency?
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.
Link API
= https://github.com/windup/windup/blob/master/reporting/api/src/main/java/
This link begins https://github.com/windup/windup/blob/master/rules-base
... you can see that they diverge after the master
sub-directory.
This was the last link I found, so I thought it would be safer to leave LinkAPI
for the 2 xrefs I had verified as is and copy-paste the complete path directory for this one, rather than truncate LinkAPI
and rewrite the other links from master
.
I've now truncated LinkAPI
, used it for all three links, and adjusted the other two links as needed.
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.
Yes you have failed to address the question
You should have shortened the {LINKAPI} attribute or created a second attribute
What happens if the repo is moved again? Then we only have to update one or two attributes
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.
@anarnold97 On Tuesday, I truncated {LinkAPI} as you suggested, so that it now can be used with all of the links to the files in the repo. I have squashed my commits, so the changes between versions of the PR aren't visible.
You are 100% correct that we should use the attribute in all locations and I have changed the PR accordingly.
@@ -105,7 +105,7 @@ endif::[] | |||
:ProductDocVscGuideURL: https://access.redhat.com/documentation/en-us/{DocInfoProductNameURL}/{DocInfoProductNumber}/html-single/visual_studio_code_extension_guide | |||
:ProductDocIntelliJGuideURL: https://access.redhat.com/documentation/en-us/{DocInfoProductNameURL}/{DocInfoProductNumber}/html-single/intellij_idea_plugin_guide | |||
:OpenShiftDocsURL: https://docs.openshift.com/container-platform/{OpenShiftProductNumber} | |||
:LinkAPI: http://windup.github.io/windup/docs/latest/javadoc/ | |||
:LinkAPI: https://github.com/windup/windup/blob/master/reporting/api/src/main/java/ |
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 link https://github.com/windup/windup/blob/master/reporting/api/src/main/java/org/jboss/windup/reporting/config/HintExists.java and the other links don't include the subdirectory javadoc
I changed the entire attribute.
:LinkAPI: now ends in ends in /java/
, not /javadoc/
.
The line I commented out refers to "Javadocs" here:
Clicking Javadocs there leads to a 404.
@@ -6,6 +6,6 @@ | |||
[id="rules-important-links_{context}"] | |||
= Additional resources | |||
|
|||
* {ProductShortName} Javadoc: http://windup.github.io/windup/docs/latest/javadoc | |||
// * {ProductShortName} Javadoc: http://windup.github.io/windup/docs/latest/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.
As this is used in line 108, lets use it here as well
// * {ProductShortName} Javadoc: http://windup.github.io/windup/docs/latest/javadoc | |
* {ProductShortName} Javadoc: https://github.com/windup/windup/blob/master/reporting/api/src/main/java |
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.
@anarnold97 The link to Javadocs (the current display text) is broken, so I think we should comment this out for now. I've alerted the windup team about the problem. Once they give us the correct link, we can add it.
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.
In which case why did you add line 108?
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.
Please address this comment
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 don't understand the question. Line 108 of document-attributes.adoc previously (MTA 6.0 and earlier) contained an attribute named :LinkAPI: that was defined as : http://windup.github.io/windup/docs/latest/javadoc/ . That definition is no longer correct and led to all the broken links. You found the correct path for two of the links (ClassificationExists and HintExists) and we defined LinkAPI based on that.
When I tried to use that attribute with IterableFilter, I saw it did not work, because IterableFilter was in a different subdirectory than the other two. I chickened out and used the full directory path, instead of truncating :LinkAPI;
You corrected me on that , so I changed :LinkAPI: and adjusted the links for those 3 items. I checked the links and they work.
You told me previously to comment out line 8 of rules-important-links after I told you the link on the website is broken.
I've searched the repo we cloned and don't see an entry for any spelling or capitalization of Javadocs/Javadox.
I've let the windup team know the link on the site is broken.
The change to line 108 of document-attributes.adoc fixes all the links except for the link in the Appendix, line 8 of rules-important-links.adoc. I can't find anything that replaces the link in the Appendix for sure, so I suggest commenting out line 8 of rules-important-links.adoc. I did change the line to include :LinkAPI: .
0ee5030
to
7e7202f
Compare
@anarnold97 Please review this PR again. Thank you. |
Still not close to being merged. |
32455a1
to
b09bb8e
Compare
Resolves https://issues.redhat.com/browse/MTA-2919 by fixing broken links in MTA 7.0 Rules development guide.
Previews:
https://file.emea.redhat.com/rhoch/links_rule_guide/html-single/#create-yaml-rule_rules-development-guide [Link in hasTags row in table in step 2]
https://file.emea.redhat.com/rhoch/links_rule_guide/html-single/#test_xml_rule_syntax [Links in section for "InterableFilter," "ClassificationExists," and "HintsExist."]
https://file.emea.redhat.com/rhoch/links_rule_guide/html-single/#yaml-rule-metadata_rules-development-guide [Section 2.1.1.4, removed an unneeded period in a bulleted list; added periods where needed in callouts]
https://file.emea.redhat.com/rhoch/links_rule_guide/html-single/#rules-important-links_rules-development-guide [Link to "Javadox" in Appendix 2.2 removed (commented out in file)]