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

Fix repo file links in the PR template #1363

Merged
merged 2 commits into from Jul 2, 2019
Merged

Conversation

@webknjaz
Copy link
Contributor

@webknjaz webknjaz commented Jul 2, 2019

Links in the PR template are broken. They start with / which effectively creates absolute URI paths relative to https://github.com. So /CONTRIBUTING.rst points to https://github.com/CONTRIBUTING.rst, not to the file view under this repo.

Fixes #1120.

See the broken template below (as rendered when I created this PR):

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 :user:<your username>"
    • 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)
@@ -5,18 +5,18 @@ much about the checklist - we will help you get started.

## Contribution checklist:

(also see [CONTRIBUTING.rst](/CONTRIBUTING.rst) for details)
(also see [CONTRIBUTING.rst](../tree/master/CONTRIBUTING.rst) for details)
Copy link
Member

@asottile asottile Jul 2, 2019

Choose a reason for hiding this comment

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

seems ../CONTRIBUTING.rst works as well (I just tried it by editing your PR)

Copy link
Contributor Author

@webknjaz webknjaz Jul 2, 2019

Choose a reason for hiding this comment

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

Interesting... I'm not sure how.

Copy link
Contributor Author

@webknjaz webknjaz Jul 2, 2019

Choose a reason for hiding this comment

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

https://github.com/tox-dev/tox/CONTRIBUTING.rst 404 Not Found for me.

../CONTRIBUTING.rst is relative to the current page https://github.com/tox-dev/tox/pull/1363 -> https://github.com/tox-dev/tox/pull/1363/../CONTRIBUTING.rst -> https://github.com/tox-dev/tox/CONTRIBUTING.rst

Copy link
Member

@asottile asottile Jul 2, 2019

Choose a reason for hiding this comment

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

iirc github renders the markdown in a "context" relative to where the file sits -- so it knows that down one directory is the CONTRIBUTING.rst file (./.github/../CONTRIBUTING.rst)

Copy link
Contributor Author

@webknjaz webknjaz Jul 2, 2019

Choose a reason for hiding this comment

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

Maybe you use some browser extension?

Copy link
Member

@asottile asottile Jul 2, 2019

Choose a reason for hiding this comment

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

oh what now it doesn't work, what did I do before 😕

Copy link
Contributor Author

@webknjaz webknjaz Jul 2, 2019

Choose a reason for hiding this comment

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

maybe you used some different view (like from the actual file page, not PR)? Also, there's some front-end caching here.

Copy link
Member

@asottile asottile left a comment

@asottile asottile merged commit a0487da into tox-dev:master Jul 2, 2019
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
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants