Skip to content
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

docs(configuration): Improve docs for curations in .ort.yml #7754

Merged
merged 3 commits into from Oct 31, 2023

Conversation

sschuberth
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (90f9993) 67.65% compared to head (24de18a) 67.65%.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #7754   +/-   ##
=========================================
  Coverage     67.65%   67.65%           
  Complexity     2044     2044           
=========================================
  Files           354      354           
  Lines         16870    16870           
  Branches       2387     2387           
=========================================
  Hits          11414    11414           
  Misses         4465     4465           
  Partials        991      991           
Flag Coverage Δ
funTest-non-docker 36.42% <ø> (ø)
test 35.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../src/main/kotlin/config/RepositoryConfiguration.kt 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

website/docs/configuration/ort-yml.md Outdated Show resolved Hide resolved
@@ -85,7 +85,7 @@ The path exclude above has the following effects:

* All projects found below the `test-data` directory are marked as excluded.
* License findings in files below the `test-data` directory are marked as excluded. This can be used in
[evaluator rules](../../tutorial/intro#6-running-the-evaluator) to for instance change the severity from error to
[evaluator rules](../../tutorial/intro.md#6-running-the-evaluator) to for instance change the severity from error to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link on the website works just fine if you test it here: https://oss-review-toolkit.org/ort/docs/configuration/ort-yml#excluding-paths

Because the tutorial is a separate instance of the docs plugin currently URL links have to be used, see the warning at the bottom of this page or this comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thought that we could merge the tutorial into the docs to avoid this issue by having only one instance of the docs plugin, what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only one instance of the docs plugin

Sorry, which docs plugin?

Anyway, you're the expert here, so whatever you say 😆 I was only addressing this as the markdown-links check failed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, which docs plugin?

The docs plugin for docusaurus, we currently use two instances of the plugin, one for the docs and one for the tutorial. So links between those two must be URL links.

Anyway, you're the expert here, so whatever you say 😆 I was only addressing this as the markdown-links check failed.

I will try to find the time to make that change in a separate PR, but until then the second commit should be removed as to my understanding it would actually break the link on the website. I wonder why the markdown lint check was failing, if this link is an issue it should also have failed before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we currently use two instances of the plugin, one for the docs and one for the tutorial.

That sounds weird... and I wasn't aware of that. Thanks for pointing that out.

I wonder why the markdown lint check was failing, if this link is an issue it should also have failed before?

Correct. Apparently some caching takes place until the particular file is touched in a PR...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the ignore pattern in the mlc_config.json needs to be changed to "^../../tutorial/intro.*$" to silence the markdown link check issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a try (with a slightly more specific pattern).

website/docs/configuration/ort-yml.md Outdated Show resolved Hide resolved
@sschuberth sschuberth requested a review from a team as a code owner October 30, 2023 19:44
@sschuberth
Copy link
Member Author

I ended up rewriting / reordering larger parts than planned as I found the docs to be more and more lacking the more often I read it.

Improve the documentation to better explain why both `curations` and
`packageConfigurations` have nested `LicenseFindingCuration`s: Their
scopes are different (project vs. package).

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Clearly separate curations from package configurations, like it is done
for global ORT configuration, for a better overview.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
@sschuberth sschuberth dismissed tsteenbe’s stale review October 31, 2023 20:50

Comment was addressed.

@sschuberth sschuberth merged commit 5b42f08 into main Oct 31, 2023
20 of 21 checks passed
@sschuberth sschuberth deleted the improve-ort-yml-curation-docs branch October 31, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants