-
Notifications
You must be signed in to change notification settings - Fork 16
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
Documentation -divided the README file into parts #109
Documentation -divided the README file into parts #109
Conversation
bartoszbicz96
commented
Oct 22, 2020
- Created new files of documentation added to new folder documentation.
- Modified README file.
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.
Thanks for the PR! A few comments regarding the split of this readme into seperate files.
After these are resolved, the next step would be to include these new asciidocs in the wiki. For that, we need "Home.asciidoc", "_Sidebar.asciidoc" and "_Footer.md".
- There isn't really any content yet for "Home" except for this one sentence that is written in the readme under "sonar-devon4j-plugin". You can simply paste this sentence in there as a placeholder. If you want you can further this description already, otherwise we can still extend it later.
- The footer can simply be copied from the documentation folder of other devonfw repos
- The sidebar contains all links to the docs you just created. See the sidebar files of other repos for reference.
- One more comment to the Wiki: There already is some content there written by @hohwille. I would propose to put the setup guide into its own file (guide-setup.asciidoc for example) and discard the other two pages, as they are basically empty.
After the wiki is complete, we need to think about which parts of our documentation should be added to the devonfw guide and create a master document which contains those chapters.
First focus on making the Wiki work and we will then proceed onto this master doc.
documentation/3rd Party Rules.adoc
Outdated
@@ -0,0 +1,12 @@ | |||
=== 3rd Party Rules |
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.
Could you rename the .adoc files like they are in the devon4j documentation?
- E.g. this file could be called "rules-thirdparty.adoc", the file for architecture rules could be called "rules-architecture.adoc", etc.
- Rename "devonfw Java Quality Profile.adoc" to "qualityprofile.adoc"
- "Installation.adoc" and "Configuration.adoc" can be refactored to lower-case
@@ -6,132 +6,3 @@ image:https://travis-ci.com/devonfw/sonar-devon4j-plugin.svg?branch=master["Buil | |||
|
|||
Plugin for https://sonarqube.org[SonarQube] to validate https://github.com/devonfw/devon4j/blob/develop/documentation/coding-conventions.asciidoc#packages[devon4j architecture]. | |||
|
|||
== Motivation |
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 newly created .adoc files should be referenced with a link in the readme. See readme of devon4j
== Architecture Rules | ||
|
||
The following image illustrates the devonfw architecture rules. The arrows show the allowed dependencies in green, discouraged dependencies in orange and forbidden dependencies in red. | ||
image:DevonArchitectureRules.png["Devon Architecture Rules",link=DevonArchitectureRules.png] |
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.
Careful with pictures, you should relocate them into the documentation folder so they can be found from the asciidoc. I would recommend creating a seperate folder for images inside the documentation folder. The value of 'link' would then be "images/[name_of_picture].png"
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 documents for installation and configuration should be renamed to lowercase, and there are two other small comments in the readme, but looking great otherwise!
I was also thinking if we should maybe change the levels of the rule chapters. The way it is right now, "Architecture Rules" is on level 2 and all the rule implementations are on level 3. Of course all our rules are checking for architecture, but IMO only the chapters "Component", "Layer" & "Scope" should be subchapters of "Architecture Rules". All the other level 3 chapters aren't really related to these dependency issues that are shown in the "DevonArchitectureRules.png".
My suggestion would be to have all rule chapters on level 2 except for component, scope, and layer rules. Maybe even rename the chapter "Architecture Rules" to "Architectural Dependency Rules" or "Dependency Rules"? WDYT @hohwille ..?
README.adoc
Outdated
* link:documentation/motivation.adoc[Motivation] | ||
* link:documentation/installation.adoc[Installation] | ||
* link:documentation/configuration.adoc[Configuration] | ||
* link:documentation/motivation.adoc[Motivation] |
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.
Motivation is referenced twice here
README.adoc
Outdated
* link:documentation/motivation.adoc[Motivation] | ||
* link:documentation/qualityprofile.adoc[devonfw Java Quality Profile] | ||
|
||
=== Rules |
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 keep the order of the rules the same way as they are in the current readme. The first three chapters are fine, but the fourth should be "Scope", then "Package", "3rd Party", etc.
Also there seems to be some merge conflict here, could you resolve this @bartoszbicz96 ? |
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.
Just a few comments regarding the levels of our chapters, but otherwise it's really starting to come together, thanks!
README.adoc
Outdated
|
||
== Installation | ||
* link:documentation/rules-architecture.adoc[Architecture Rules] |
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 level for this bullet is fine, but I would indent the next three bullet points one further using **
instead of *
. So that the chapters "Layer", "Component" & "Scope" are subchapters of "Architectural Dependency Rules".
README.adoc
Outdated
|
||
This `sonar-devon4j-plugin` provides a plugin extending https://sonarqube.org[SonarQube] with the ability to validate your Java code according to the devon4j architecture. | ||
== Architectural Dependency Rules |
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.
All our rules are "Architecture Rules", so I would put this as a header here. Then the first bullet point under this header could be "Architectural Dependency Rules" with its three subchapters "Layer", "Component" & "Scope".
So basically: Put "Architecture Rules" here, and put "Architectural Dependency Rules" as the first bullet point with a link to the document. Also don't forget to rename the document and its title, so that everything is consistent.
README.adoc
Outdated
If you have https://sonarqube.org[SonarQube] installed, you only need to go to its marketplace and install the latest version of this `sonar-devon4j-plugin`. | ||
Further you need to import the devonfw quality profiles (or activate all the rules of this plugin in your quality profile). | ||
For further details read the https://github.com/devonfw/sonar-devon4j-plugin/wiki/guide-sonar-qube-setup[SonarQube setup guide]. | ||
== Implementation Rules |
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 think we need to put these under a seperate header, as they are also in some way architectural rules. Just put the next four bullets as level one bullet points under the header "Architecture Rules".
documentation/Configuration.adoc
Outdated
@@ -0,0 +1,18 @@ | |||
== Configuration |
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.
Could you rename this file and "Installation.adoc" to lowercase?
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 i made it... anyway i will try to fix it
895988a
to
810c4db
Compare
documentation/rules-component.adoc
Outdated
@@ -0,0 +1,14 @@ | |||
=== Layer Rules |
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.
This doc should contain the component rules
@@ -0,0 +1,13 @@ | |||
=== Naming Convention Rules |
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.
All headers for rule documents should be on level two, so starting with "==", except for component, layer and scope rules, to be consistent with the changes from your last commits.
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.
IMHO we should follow the general best-practices from devonfw:
Simply start any AsciiDoc file on Level1.
When including the documents in the main AsciiDoc-file for the PDF set [leveloffset=«number»]
to increase the level so that the result fits to our needs.
26ff0a1
to
810c4db
Compare
941d3b6
to
06eb86d
Compare
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.
Looks great now! I will approve this and then we can continue with the integration of this new documentation into the devonfw documentation pipeline in a seperate PR.
@bartoszbicz96 thanks for this PR. Further we should also consider document generation for this repo like we have for all other projects. |
@hohwille |
Thanks for the explanation. I got the rationale now and indeed we agreed in devonfw to put all documentation into a
Would it be possible to update this PR accordingly? |
Perfect, thanks for the pointers. Sure, I will connect with Bartosz and we will get on the tasks. So just to clarify: Is the readme fine as it is in this PR? |
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.
Thanks for taking this further. However, this still can not work. Have you tried to build it and check if the PDF gets generated and installed into maven repo? IMHO this can not work anymore after removing the docgen parent.
documentation/pom.xml
Outdated
|
||
<groupId>com.devonfw.tools.sonar-devon4j-plugin</groupId> | ||
<artifactId>devonfw-docgen-pdff</artifactId> |
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.
Looks like a typo and this does make sense to me. Why dont we still inherit this from devonfw-docgen-pdf
?
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 it is OK to change the README to link to the actual contents in the documentation folder.
Further, the README is "just" the starting point for documentation when people directly go to this repo on github.
For the PDF we would need an additional file that uses include
tags rather than link
s.
I have the following observations and suggestions for improvement:
-
IMHO splitting all the subsections of the rules into individual asciidoc file is too fine grained.
In the most extreme case users have to click a link to get one sentence of information:
https://github.com/devonfw/sonar-devon4j-plugin/blob/cd966e96ee18f815669611c73d81d38149561071/documentation/rules-package.adoc
I would instead keep these rules stuff all in one asciidoc file. The other splitting in configuration, installation, etc. makes perfectly sense to me. -
After the restructuring the image is not displayed anymore:
https://github.com/devonfw/sonar-devon4j-plugin/blob/cd966e96ee18f815669611c73d81d38149561071/documentation/rules-architecturaldependency.adoc
IMHO this happened because the newline was removed before theimage
tag. It is important to fix this. Esp. for the PDF the image should be embeded rather than an external link. -
As already agreed we should add the PDF generation so it gets deployed alongside with every release.
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.
IMHO now ready to be merged. Thanks to all who contributed. 👍