Skip to content

Conversation

@adineh
Copy link

@adineh adineh commented Sep 19, 2023

Reference to a related issue in the repository
#27

Add a description
Making the project reamde file more attractive in promotions.

Some questions to ask
What is this change? improving the project readme file.
What does it fix? the content and structure of the project readme file.
Is this a bugfix or a feature? Just enhancement of the readme file
Does it break any existing functionality or force me to update to a new version? no
How has it been tested? by markdownlint

Take this checklist as orientation for yourself, if this PR is ready for Maintainer Review

  • My suggestion follows the governance rules.
  • All commits of this PR are signed.
    • I found this requirement on pull request.
  • My changes generate no errors when passing CI tests.
  • I updated all documentation (readmes incl. figures) according to my changes.
  • I have successfully implemented and tested my fix/feature locally.
  • Appropriate reviewer(s) are assigned.

If you are still working on this PR, submit as "Draft Pull Request", which can be selected in the green button at the bottom.

If your work is done, but you can’t check all the boxes above, please explain why.
If all boxes are checked or commented, you can submit as a pull request directly.

@adineh adineh requested a review from a team September 19, 2023 11:41
adineh and others added 3 commits September 19, 2023 13:50
More information and about sub-libraries and maintainers has been added into the readme file. It also, somehow enhanced to be more readable and categrized.

Signed-off-by: Hadi Adineh <122263902+hadi-adineh-ascs@users.noreply.github.com>
Signed-off-by: Hadi Adineh <hadi.adineh@asc-s.de>
Signed-off-by: Hadi Adineh <hadi.adineh@asc-s.de>
Signed-off-by: Hadi Adineh <hadi.adineh@asc-s.de>
@adineh adineh force-pushed the hadi-adineh-ascs-README-update branch from e51088e to 0980b78 Compare September 19, 2023 11:51
@jdsika
Copy link
Contributor

jdsika commented Sep 20, 2023

@ClemensLinnhoff do you know why the linter is complaining even if the links are clickable?

@jdsika jdsika added the documentation Improvements or additions to documentation label Sep 20, 2023
@jdsika jdsika marked this pull request as ready for review September 20, 2023 07:07
Signed-off-by: ClemensLinnhoff <Clemens.Linnhoff@partner.bmw.de>
Signed-off-by: ClemensLinnhoff <Clemens.Linnhoff@partner.bmw.de>
Signed-off-by: ClemensLinnhoff <Clemens.Linnhoff@partner.bmw.de>
@ClemensLinnhoff
Copy link
Contributor

@ClemensLinnhoff do you know why the linter is complaining even if the links are clickable?

The linter seems to be confused, because the heading is also a link. It does not fail at the link to ## SL1 - Perception Sensor Models but it fails at the link to #### [sl-1-0-sensor-model-repository-template](https://github.com/openMSL/sl-1-0-sensor-model-repository-template).

I guess this is not really a proper heading style. I originally thought the reason is, that the heading is also part of a list (which I wouldn't do) but I tested it and the linter still fails.

There are now three possibilities on how to proceed:

  1. Ignore the linter warnings and merge anyways.
  2. Add MD051 to a list of Markdown rules to be ignored. (I wouldn't do that, because it is actually a good check)
  3. Not use links in headings.

Since linking from the table of contents to the heading works and I think it is kind of intuitive, that the heading directly links to the corresponding repository, I would suggest we choose option 1 and simply ignore the linter warnings.

All that aside, I would still not use a list for the headings. Therefore, I left my commit testing this on SL1-0 in the PR.

@ClemensLinnhoff
Copy link
Contributor

Furthermore, I would not put the images from the repository in the profile, since it makes it quite long to scroll through.

Signed-off-by: Hadi Adineh <hadi.adineh@asc-s.de>
@adineh
Copy link
Author

adineh commented Oct 5, 2023

@ClemensLinnhoff: Due to your review comments, following changes has been done in last commit:
- Links removed from Headings
- Headings are no longer parts of lists
- Some white spaces and empty lines removed
- Images has been removed

Signed-off-by: Hadi Adineh <hadi.adineh@asc-s.de>
@ClemensLinnhoff
Copy link
Contributor

@jdsika and I just created a brand new documentation page for OpenMSL: https://openmsl.github.io/doc/

I think we can integrate some of your improvements intended for the profile there. But we can discuss this in the next maintainer meeting.

@ClemensLinnhoff
Copy link
Contributor

I think we can close this PR and the corresponding issue as some of the proposed changes were implemented in the documentation instead of the profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants