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

Provide an explanatory message when attempted command parsing produces an empty result #2799

Closed
wants to merge 32 commits into from
Closed

Provide an explanatory message when attempted command parsing produces an empty result #2799

wants to merge 32 commits into from

Conversation

jayaddison
Copy link
Contributor

This attempts to clarify the error messaging when the install_command entry in a tox environment configuration is empty.

  • ran the linter to address style issues (tox -e fix)
  • wrote pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

Continues-from #2715.

Relates to #2695, #2798.

…rsing results are empty

(rationale: the input was of the correct type: string, it's the contents (value) of the string that caused issues)
…vironment -- that involves accessing and parsing the 'install_command'
…t element) to 'pip install' invocation, so that installation is attempted
…ion statement into pytest.raises 'match' argument
…o hold parametrized argument names"

This reverts commit 4fce79c.
This reverts commit ff5d70c.
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.

Please allow pushes to this PR.

@jayaddison
Copy link
Contributor Author

Maintainer edits should be allowed @gaborbernat (looks like you were able to push some commits, let me know if I can help)

@gaborbernat gaborbernat closed this Jan 2, 2023
@gaborbernat
Copy link
Member

ERROR: Permission to openculinary/tox.git denied to gaborbernat.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

seems not

@gaborbernat
Copy link
Member

Why there's like 30 commits here?

@jayaddison
Copy link
Contributor Author

Sometimes (often, recently) I'm a bit tedious about trying to keep the entire commit history for a pull request after it's open (even if it means including reverts, rather than squash committing).

It's kinda about being open about errors and mistakes during development. But yep, I do worry that it's annoying to read/review, and wonder how to figure out the balance (per-project/community).

I can reduce the commits down if that'd be useful (to a single commit, or to a few logical grouped changes).

@gaborbernat
Copy link
Member

Replaced with #2807

@gaborbernat
Copy link
Member

Also, please never use merge to make the branch up to date. I always prefer to rebase onto the main, which makes it easier to read and follow.

@jayaddison
Copy link
Contributor Author

Ok, understood 👍 I'll do my best to remember not to do that again here.

@jayaddison jayaddison deleted the issue-2695/empty-command-parsing-improve-error-messaging branch January 2, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants