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

Add hint of Issue #478 to error text #2081

Closed
wants to merge 0 commits into from

Conversation

DuncanBetts
Copy link
Contributor

@DuncanBetts DuncanBetts commented Nov 24, 2016

  • [ master ] Target: for bug or doc fixes, target master; for new features, target features;

  • [ Not Done Yet ] Add yourself to AUTHORS

  • [ Not Done Yet ] Add a new entry to CHANGELOG.rst

RST Formatted, as requested:
Issue 478 <https://github.com/pytest-dev/pytest/issues/478>

[Issue 478] (#478)

I have implemented enkore's suggestion.

Feedback, suggestions, and rejection, all welcome! This is my first pull request 🎉.

Thanks @DuncanBetts for the PR (https://github.com/DuncanBetts).

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0008%) to 92.817% when pulling ba3c1d2 on DuncanBetts:master into 38f7562 on pytest-dev:master.

@nicoddemus
Copy link
Member

This is my first pull request 🎉

Congrats on your first PR! 🎈

The fix looks good! Could you also add yourself to AUTHORS and a new CHANGELOG entry?

Thanks!

@@ -11,7 +11,7 @@
* Cope gracefully with a .pyc file with no matching .py file (`#2038`_). Thanks
`@nedbat`_.

*
* Add hint to error message hinting possible missing __init__.py (`#478`_). Thanks `@DuncanBetts`_.
Copy link
Member

Choose a reason for hiding this comment

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

You need to add the actual link to your user, see the lines below for @nedbad_:

.. _@DuncanBetts: https://github.com/DuncanBetts

(Add it before nedbat's to keep the list sorted)

Copy link
Contributor Author

@DuncanBetts DuncanBetts Nov 24, 2016

Choose a reason for hiding this comment

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

Ok. Am about to eat so will do tomorrow now. Thanks :).

@DuncanBetts
Copy link
Contributor Author

I committed the AUTHORS and CHANGELOG in a separate commit, and then ran interactive rebase and fixed up/squashed the new commit into the old commit, which has changed the SHA I believe. When I tried to push I was behind HEAD, pull resulted in a 3 way merge. I think I did the right thing?

@nicoddemus
Copy link
Member

When I tried to push I was behind HEAD, pull resulted in a 3 way merge. I think I did the right thing?

Yes no problem. If you want to avoid the merge, use git fetch and git rebase upstream/master first so you will rebase on the most up-to-date version of the repository (assuming you added pytest-dev/pytest as a remote named upstream). Then you should be able to git push cleanly.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0008%) to 92.817% when pulling f05bc4f on DuncanBetts:master into 38f7562 on pytest-dev:master.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

lovely, we can merge after the text fixup 👍

@DuncanBetts
Copy link
Contributor Author

I followed your advice @nicoddemus, it still triggered a fresh three way merge and there's an extra line in the CHANGELOG as a result.
Most likely because in an effort to keep this to a single commit, I used commit --amend.
Will try and fix now.

@nicoddemus
Copy link
Member

nicoddemus commented Nov 25, 2016

Hmm that's strange, it should have applied cleanly. Here are the steps in case they were not clear from my previous explanation:

  1. Add the official repository as upstream:
$ git remote add upstream https://github.com/pytest-dev/pytest
  1. Follow this steps to rebase:
$ git fetch --all
$ git rebase upstream/master

If you get a conflict somewhere, fix the conflict by following the instructions from git.

(Sorry if I'm stating the obvious, I don't know your level of expertise on git 😁 )

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0008%) to 92.819% when pulling 438911f on DuncanBetts:master into 33796c8 on pytest-dev:master.

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.

None yet

4 participants