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

Feature/package revision in json info #7890

Merged

Conversation

sdmg15
Copy link
Contributor

@sdmg15 sdmg15 commented Oct 16, 2020

Changelog: Feature: Including package revision information in output from conan info (when revisions are enabled).
Docs: Omit

  • Refer to the issue that supports this Pull Request.
    closes [feature] 'conan info' to return the package revision #7796
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for your contribution.

Please check the review, and try to add a check in the existing InfoTest.test_json_info_outputs() (conans/test/functional/command/info/info_test.py)

conans/client/conan_command_output.py Outdated Show resolved Hide resolved
conans/client/conan_command_output.py Outdated Show resolved Hide resolved
@sdmg15 sdmg15 requested a review from memsharded Oct 19, 2020
@@ -177,8 +179,8 @@ def _add_if_exists(attrib, as_list=False):
if isinstance(ref, ConanFileReference):
item_data["recipe"] = node.recipe

if get_env("CONAN_CLIENT_REVISIONS_ENABLED", False) and node.ref.revision:
Copy link
Member

Choose a reason for hiding this comment

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

I am reviewing this, seems unused now.

@@ -106,6 +106,7 @@ def _print(it_field, show_field=None, name=None, color=Color.BRIGHT_GREEN):
_print("recipe", name="Recipe", color=None)
if show_revisions:
_print("revision", name="Revision", color=None)
_print("package_revision", name="Package revision", color=None)
Copy link
Member

Choose a reason for hiding this comment

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

Printing the package revision to screen too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the assertNotIn I did is the test not worth it ?

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that it was asserting that the revisions are not defined, the opposite of what we want. We want the revisions to be part of the information. That test now checks for self.assertEqual() to check that it is both there and has a value, instead of checking that it is not defined.

@memsharded memsharded added this to the 1.31 milestone Oct 20, 2020
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Hi @sdmg15

I think this is ready for another review, and we can take it out of draft. Assigning it tentatively for next Conan 1.31 release

@memsharded memsharded marked this pull request as ready for review Oct 20, 2020
@memsharded memsharded merged commit 9cf6a59 into conan-io:develop Oct 20, 2020
2 checks passed
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.

[feature] 'conan info' to return the package revision
3 participants