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

Add support to declare multiple README files #118

Merged

Conversation

wagnerluis1982
Copy link
Contributor

@wagnerluis1982 wagnerluis1982 commented Dec 12, 2020

Resolves: python-poetry/poetry#873


This code reuses the "readme" entry in a way to allow the user to declare in the old way or the new way. The two following declarations become valid:

Single file

readme = "README.rst"

Multiple files

readme = [
    "README.rst",
    "HISTORY.rst"
]

If the user declares files in different formats, the strict validation will issue.

@sinoroc
Copy link

sinoroc commented Dec 12, 2020

Where should I add documentation? Should I open another MR in the main project for that?

I think so, yes.

@wagnerluis1982
Copy link
Contributor Author

I rebased my branch to latest master and solved conflicts. Also added documentation to python-poetry/poetry#4743

poetry/core/factory.py Outdated Show resolved Hide resolved
poetry/core/factory.py Outdated Show resolved Hide resolved
poetry/core/factory.py Outdated Show resolved Hide resolved
poetry/core/json/schemas/poetry-schema.json Outdated Show resolved Hide resolved
poetry/core/masonry/metadata.py Outdated Show resolved Hide resolved
poetry/core/packages/package.py Outdated Show resolved Hide resolved
poetry/core/utils/helpers.py Outdated Show resolved Hide resolved
tests/masonry/builders/test_builder.py Outdated Show resolved Hide resolved
tests/test_factory.py Show resolved Hide resolved
@wagnerluis1982 wagnerluis1982 force-pushed the feature/multiple-readme-files branch from 789bfad to 6ec8c66 Compare November 15, 2021 00:05
Copy link
Contributor Author

@wagnerluis1982 wagnerluis1982 left a comment

Choose a reason for hiding this comment

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

Thanks for the awesome suggestions.

I added my fixes to the code: please note that I rebased the branch to reduce the number of commits.

Comments about my fixes:

  • I chose isinstance(config["readme"], list) as the TOML parser only delivers list-like objects, no need to check for tuple.
  • About using a Sequence, please check my separate comment
  • Pluralized Package.readme (became readmes) and readme_type => description_type
  • Added a test for validation fail when readme types mismatch
  • Refactoring according to the comments
  • Fixed a few glitches I found on the way

poetry/core/masonry/metadata.py Outdated Show resolved Hide resolved
@wagnerluis1982 wagnerluis1982 force-pushed the feature/multiple-readme-files branch from 6ec8c66 to ce15451 Compare November 15, 2021 06:49
neersighted
neersighted previously approved these changes Nov 15, 2021
@neersighted neersighted merged commit f101162 into python-poetry:master Nov 15, 2021
@wagnerluis1982 wagnerluis1982 deleted the feature/multiple-readme-files branch November 15, 2021 07:19
abn added a commit to abn/poetry-core that referenced this pull request Nov 15, 2021
@abn abn mentioned this pull request Nov 15, 2021
abn added a commit to abn/poetry-core that referenced this pull request Nov 15, 2021
abn added a commit that referenced this pull request Nov 15, 2021
wagnerluis1982 added a commit to wagnerluis1982/poetry that referenced this pull request Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can readme section conains several files?
3 participants