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

🔥 Cut comments off dependency lines #1262

Merged

Conversation

webknjaz
Copy link
Contributor

@webknjaz webknjaz commented Apr 15, 2019

This enables users to have inline comments in deps lines in tox.ini. In particular, this allows having pyup bot integration.

Ref: pyupio/dparse#34
Ref: ansible/molecule#1973

Fixes #1260

Contribution checklist:

(also see CONTRIBUTING.rst for details)

  • wrote descriptive pull request text
  • added/updated test(s)
  • updated/extended the documentation
  • added relevant issue keyword
    in message body
  • added news fragment in changelog folder
    • fragment name: <issue number>.<type>.rst for example (588.bugfix.rst)
    • <type> is must be one of bugfix, feature, deprecation,breaking, doc, misc
    • if pr has no issue: consider creating one first or change it to the pr number after creating the pr
    • "sign" fragment with "by @"
    • please use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files - by :user:superuser."
    • also see examples
  • added yourself to CONTRIBUTORS (preserving alphabetical order)

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Add tests, changelog entry 👍

src/tox/config/__init__.py Outdated Show resolved Hide resolved
@webknjaz
Copy link
Contributor Author

@gaborbernat is it fine now?

Copy link
Contributor

@asottile asottile left a comment

Choose a reason for hiding this comment

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

I'm ok signing off on this, I'll let @gaborbernat make the final call though

@gaborbernat
Copy link
Member

I'll merge it after we fix our CI.

@webknjaz
Copy link
Contributor Author

webknjaz commented Apr 15, 2019

@asottile @gaborbernat thanks!

@gaborbernat
Copy link
Member

Please do specify/document the exact behaviour in config.rst under deps.

@webknjaz
Copy link
Contributor Author

okay

@decentral1se
Copy link

Thanks for picking this up @webknjaz 🚀

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

As said above document deps comment behaviour.

@gaborbernat
Copy link
Member

master fixed now

@webknjaz webknjaz force-pushed the bugfix/rm-comments-from-dep-lines branch from 43ddf99 to 6410943 Compare April 16, 2019 20:44
docs/config.rst Outdated Show resolved Hide resolved
@gaborbernat gaborbernat force-pushed the bugfix/rm-comments-from-dep-lines branch 5 times, most recently from 335f248 to 9783de2 Compare April 17, 2019 11:39
@gaborbernat gaborbernat force-pushed the bugfix/rm-comments-from-dep-lines branch from 9783de2 to c3da4b1 Compare April 17, 2019 12:47
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

seal of app

@gaborbernat gaborbernat merged commit 3c6b4f2 into tox-dev:master Apr 17, 2019
@webknjaz
Copy link
Contributor Author

Thanks! Can I expect a release soon?

@gaborbernat
Copy link
Member

Not much content for a new release 🤔 but why not!

@webknjaz
Copy link
Contributor Author

I'd appreciate that since it'd unblock issues in Molecule :)

@gaborbernat
Copy link
Member

#1265

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.

Dependency entries with comments are not parsed correctly
4 participants