Skip to content

issue469: junit parsing nodeid, add method test #1432

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

Merged
merged 1 commit into from
Mar 3, 2016

Conversation

tomviner
Copy link
Contributor

@tomviner tomviner commented Mar 3, 2016

Additional test as recommend before #1431 was merged :-)

Also added name to the changelog entry. And repositioned the rst link definition.

While I'm here couple comments on the changelog:

  • we're in risk of the (#469_) numbers referencing both issues and pull requests clashing (both are up to around 1400 now). Obviously all links are explicitly defined, but could be confusing. rst-lint will error on duplicate definitions, but if someone forgets the 2nd definition, it would silently reference the first. Maybe reference PRs as (PR#469_) and keep (#469_) for issues?
  • minor point, but all the trailing whitespace makes editing the changelog difficult as my editor strips it. Would someone be happy stripping it? Or maybe my editor settings are overzealous :-)

@The-Compiler
Copy link
Member

we're in risk of the (#469_) numbers referencing both issues and pull requests clashing (both are up to around 1400 now). Obviously all links are explicitly defined, but could be confusing. rst-lint will error on duplicate definitions, but if someone forgets the 2nd definition, it would silently reference the first.

I don't understand how they'd clash - both PRs and issues draw from the same "number pool", so there never will be a PR with the same number as an issue.

@tomviner
Copy link
Contributor Author

tomviner commented Mar 3, 2016

both PRs and issues draw from the same "number pool"

@The-Compiler ahhh, that's a most satisfying explanation for why the numbers are so close! Ignore my point then 😁

@hpk42
Copy link
Contributor

hpk42 commented Mar 3, 2016

what about putting the links inline so one can easily click on it when reading the text version of the changelog? (yes, i often do that)

@nicoddemus
Copy link
Member

Thanks @tomviner! 😁

@hpk42 no problem... that might even help avoiding merge conflicts, as each entry will be self contained. Will change that on the next opportunity and see how that goes.

nicoddemus added a commit that referenced this pull request Mar 3, 2016
issue469: junit parsing nodeid, add method test
@nicoddemus nicoddemus merged commit c8ca1d1 into pytest-dev:master Mar 3, 2016
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.

4 participants