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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update documentation for v32 #3292

Merged
merged 14 commits into from
Apr 14, 2023
Merged

Update documentation for v32 #3292

merged 14 commits into from
Apr 14, 2023

Conversation

AyanSinhaMahapatra
Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra commented Mar 20, 2023

Fixes #3277
Fixes #3278
Fixes #3298

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 馃搼 and links the original issue above 馃敆
  • Tests pass -- look for a green checkbox 鉁旓笍 a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 馃搧

Looks like there was an issue in docutils/sphinx-rtd-theme so we
needed to downgrade to docutils < 0.17 for the unordered lists to
be rendered again properly.

Reference: readthedocs/sphinx_rtd_theme#1115
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra marked this pull request as draft March 20, 2023 16:23
Refernce: #3278
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra marked this pull request as ready for review March 27, 2023 09:18
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
- Add new console script `regen-package-docs`
- Add new shell script `regen_package_docs.sh`
- Add new jinja template to generate docs using data
- Add this script on makefile to run automatically on doc generation
- Move explanations -> reference section

Reference: #3298
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
- Add doc check reminder in PR template
- Delete stray file
- Misc license related updates to align docs

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra changed the title Update docmuentation for v32 Update documentation for v32 Mar 28, 2023
Updating the column order in available package
parsers from feedback.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
AyanSinhaMahapatra and others added 3 commits April 10, 2023 11:58
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@AyanSinhaMahapatra
Copy link
Member Author

Btw, doc builds for this branch are enabled and can be viewed here: https://scancode-toolkit.readthedocs.io/en/doc-update-licenses/

README.rst Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

docs/source/cli-reference/basic-options.rst

Lines 89-90: "If we run license detection (with --license-text) on the above text"

Perhaps put "--license-text" inside backticks? In my view of the RTD page, that flag is displayed with a single hyphen/dash, "-license-text", even though when copied and pasted from the RTD page to, e.g., Notepad+ I see a double-dash.

Line 124: change "occurances" to "occurrences"

Lines 669-670: In the code we have these 2 URLs, which look like markup inside this .rst file:

    [`WTFPL`](http://www.wtfpl.net/txt/copying/) and
    [`MIT`](https://opensource.org/licenses/MIT).

and in the RTD they show up like this (no idea how this will be displayed in my comment):

        [`WTFPL`](http://www.wtfpl.net/txt/copying/) and
        [`MIT`](https://opensource.org/licenses/MIT).

Maybe we want this instead?

        `WTFPL <http://www.wtfpl.net/txt/copying/>`_ and
        `MIT <https://opensource.org/licenses/MIT>`_.

Line 673: In "we have to following license detection result" replace "to" with "the"

Lines 748-752: In this passage:

    Here from the ``"detection_log": ["unknown-intro-followed-by-match"]`` added diagnostics
    information we get to know that there was a unknown intro license match, followed by
    proper detections, so we concluded the unknown intro to be a introduction to the
    following license and hence concluded the license from the license matches after the
    unknown detection.

consider

  • replacing "get to know" with "learn"
  • replacing "a unknown" with "an unknown"
  • replacing "so we concluded" with "so we conclude"
  • replacing "a introduction" with "an introduction"
  • replacing "and hence concluded" with "and hence conclude"

Copy link
Member Author

Choose a reason for hiding this comment

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

WTFPL

This is just an example file where this was markdown, we did not want a link here. So this is ok.
All other comments addressed.

Copy link
Member

Choose a reason for hiding this comment

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

docs/source/cli-reference/scan-options-post.rst

Line 32: Change 'Their' to 'their' in Then, the following directories are marked as "Source", i.e. Their "is_source" attribute is set

Line 33: Change to "True", as they contain source code majorly. to to "True", as they contain mostly source code.

Line 56: In scancode-toolkit as top level packages, dependenices and licenses change dependenices to dependencies

Line 57: Change now provide a improved consolidated data. to now provide improved consolidated data.

Line 206: such as the ScanCode LicenseDB -- is that the current name of the DB?

Line 252: What does this mean?

It whole JSON file is structured as follows, when it has "license_clarity_score"::

Does this capture the intent?

When the "license_clarity_score" is included, the entire JSON file is structured as follows::

Line 349: change seperately to separately

Line 479: change comparision to comparison

Line 600: change dependenices to dependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

such as the ScanCode LicenseDB -- is that the current name of the DB?

Yes! See https://github.com/nexB/scancode-licensedb#scancode-licensedb and https://scancode-licensedb.aboutcode.org/

Addressed all comments otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

docs/source/contribute/contrib_doc.rst

Lines 319-323: Each of these 3 links takes me to a 404 page:

It's possible to generate docs automatically from data by using a combination of:

  • shell scripts: example <https://github.com/nexB/scancode-toolkit/blob/develop/docs/scripts/regen_package_docs.sh>_
  • python scripts: example <https://github.com/nexB/scancode-toolkit/blob/develop/src/packagedcode/regen_package_docs.py>_
  • jinja templates: example <https://github.com/nexB/scancode-toolkit/blob/develop/src/packagedcode/templates/available_package_parsers.rst>_

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, these would be working when we merge this PR :P As they are in this PR actually.

Copy link
Member

Choose a reason for hiding this comment

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

docs/source/how-to-guides/add_new_license.rst

Line 9: In To add new license insert a before new.

Line 15: In numbers from 0 to 9,and add a space after the comma.

Line 18: In less than 50 characters change less to fewer.

Line 20: In "We also have to add a spdx_license_key" change a to an.

Line 24: In I.e. the filename change I.e. to For example,.

Line 30: Change characters, copyrights to characters (e.g., copyrights).

Line 33: In the data attributes for the license in YAML format as YAML frontmatter., what does as YAML frontmatter mean?

Line 46: Change but nice to have always to but always nice to have.

Line 54: Change which are very similar to that are very similar.

Line 55: Change make sure we match correctly these licenses to make sure we match these licenses correctly.

Line 79: Change developement to development.

Line 80: Change to make sure we reindex the licenses and this validates the new licenses. to to make sure we reindex the licenses (and this also validates the new licenses).

Copy link
Member Author

Choose a reason for hiding this comment

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

See https://github.com/eyeseast/python-frontmatter, also added this link here. It's a way of adding metadata to plain text files. We recently added this to half the number of files under data/licenses/

Comments addressed, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

docs/source/how-to-guides/add_new_license_detection_rule.rst

Line 32: Change this name, for example to this name; for example,.

Line 35: In Save your rule text in this file, if there are specific words, change , to ;.

Line 64: Change Each rules needs have one flag to describe the type of license rule, the options are: to Each rule needs to have one flag to describe the type of license rule. The options are:`

Line 86: Change to make sure we reindex the rules and this validates the new licenses. to to make sure we reindex the rules (and this also validates the new licenses).

Copy link
Member Author

Choose a reason for hiding this comment

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

Not adding the last parenthesis, as it's not a side effect, that is the reason why we run the reindexing.

Addressed other comments, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

docs/source/reference/available_package_parsers.rst

Line 8: Scancode supports -- let's try to be consistent with our product naming -- at least change this to ScanCode, although it's actually ScanCode Toolkit, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Chnaged to ScanCode. It's both scancode.io and scancode-toolkit, as scancode.io also uses toolkit and this is also what is supported there.

Copy link
Member

Choose a reason for hiding this comment

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

docs/source/rst_snippets/note_snippets/post_license_policy.rst

Lines 6-9: The full passage:

We do not have
licenses as an required option because this plugin would be
upgraded to also include the license policy attribute for
packages too.

  • Change an required option to a required option.
  • I don't understand what we are trying to say with this portion of the passage:

because this plugin would be
upgraded to also include the license policy attribute for
packages too.

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 could have made the --license option a required option to run --license-policy, but that's not the case because in the future, this would also include packages.

@johnmhoran
Copy link
Member

@AyanSinhaMahapatra I've completed my review of the 59 files.

@AyanSinhaMahapatra
Copy link
Member Author

@johnmhoran thanks a lot for the feedback and comments, they were really helpful in making the documentation better, more correct and consistent. Have addressed all comments and kept some answers unresolved above for you to see.

Adds updates and fixes wrt. feedback and comments provided
by @johnmhoran

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra merged commit 9939e13 into develop Apr 14, 2023
33 checks passed
@AyanSinhaMahapatra AyanSinhaMahapatra deleted the doc-update-licenses branch April 14, 2023 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants