Skip to content

Comments

Fix ci and revert #118#235

Merged
abn merged 2 commits intopython-poetry:masterfrom
abn:revert-breackages
Nov 15, 2021
Merged

Fix ci and revert #118#235
abn merged 2 commits intopython-poetry:masterfrom
abn:revert-breackages

Conversation

@abn
Copy link
Member

@abn abn commented Nov 15, 2021

Reverting #234 because this was an incorrect change as the change meant the wrong version of poetry-core was used for testing against downstream poetry. See logs here.

Run poetry add ../poetry-core

Updating dependencies
Resolving dependencies...

Writing lock file

Package operations: 0 installs, 1 update, 0 removals

  • Updating cachecontrol (0.12.6 -> 0.12.10)
diff --git a/poetry.lock b/poetry.lock

Reverting #118 as this is a breaking change. The change needs to be reworked to support existing downstream usage. Alternatively, a change to poetry first to support list of files can also be made.

―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― test_create_poetry ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

    def test_create_poetry():
        poetry = Factory().create_poetry(fixtures_dir / "sample_project")
    
        package = poetry.package
    
        assert package.name == "my-package"
        assert package.version.text == "1.2.3"
        assert package.description == "Some description."
        assert package.authors == ["Sébastien Eustace <[email protected]>"]
        assert package.license.id == "MIT"
>       assert (
            package.readme.relative_to(fixtures_dir).as_posix()
            == "sample_project/README.rst"
        )
E       AttributeError: 'ProjectPackage' object has no attribute 'readme'

tests/test_factory.py:34: AttributeError

@abn abn force-pushed the revert-breackages branch from e2a4e01 to 220a3bb Compare November 15, 2021 09:13
@abn abn changed the title Revert #234 #118 Fix ci and revert #118 Nov 15, 2021
@abn
Copy link
Member Author

abn commented Nov 15, 2021

fyi: @neersighted @wagnerluis1982

@abn abn merged commit d7e1761 into python-poetry:master Nov 15, 2021
@abn abn deleted the revert-breackages branch November 15, 2021 09:19
@wagnerluis1982
Copy link
Contributor

@abn how should I address the rework? I mean, do you have any suggestions, apart from avoid to rename the attributes?

BTW, do you have any instant messaging channel for discussing?

@finswimmer
Copy link
Member

finswimmer commented Nov 15, 2021

BTW, do you have any instant messaging channel for discussing?

You can join our discord server.

@abn
Copy link
Member Author

abn commented Nov 15, 2021

I'm on the Poetry discord.

As suggestion, I'd say do not rename the attribute (as we are not updating the schema property name anyway). And follow a sequence like this to avoid the failure (think it's only an issue in tests). This is off the top of my head, no real validation done.

  1. Make poetry aware that package.readme can be a string or a list. Unsure if there are usages that might have an issue with this.
  2. Always listify the readme property in core.
  3. Once new release of core is cut and used by poetry, remove "is str or list" checks.

EDIT: Thinking about one aspect I am not sure about is how any other projects using core would be impacted if readme started returning a list. Unlikely though anyone is really using the readme, but worth considering.

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.

3 participants