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

TST: Standardize GitHub references #55461

Open
rhshadrach opened this issue Oct 9, 2023 · 12 comments
Open

TST: Standardize GitHub references #55461

rhshadrach opened this issue Oct 9, 2023 · 12 comments
Labels
Clean Needs Discussion Requires discussion from core team before further action Testing pandas testing functions or related to the test suite

Comments

@rhshadrach
Copy link
Member

Currently in tests we have a number of different formats for referencing GitHub issues or pull requests. The sense I gather is that there are two different formats people may like:

Short:

def test_foo():
    # GH#1234
    ...

def test_bar():
    # GH#5678
    ...

URL:

def test_foo():
    # https://github.com/pandas-dev/pandas/issues/1234
    ...

def test_bar():
    # https://github.com/pandas-dev/pandas/pull/5678
    ...

Please feel free to suggest any other alternatives as well. One option is of course to continue with the status-quo: no standardization of referencing GitHub issues.

cc @pandas-dev/pandas-core

@rhshadrach rhshadrach added Testing pandas testing functions or related to the test suite Clean Needs Discussion Requires discussion from core team before further action labels Oct 9, 2023
@mroeschke
Copy link
Member

I would prefer the URL reference moving forward

@jbrockmendel
Copy link
Member

not a huge deal but URL-based is awkward in a few cases where i want to grep: 1) for # TODO(long_url_here): ..., 3), need to grep for the "pull" or "issue" variants, 3) possible ambiguity of if http:// is included. i generally only use the URL if im referencing a specific comment in the thread.

that said, the upside of standardizing outweighs those complaints so i wont object if thats what people prefer.

@phofl
Copy link
Member

phofl commented Oct 9, 2023

I prefer short

@bashtage
Copy link
Contributor

bashtage commented Oct 9, 2023

I would also vote for short. Probably # GHXXXX. I find the second # unnecessary.

@rhshadrach
Copy link
Member Author

If we go on votes alone, it's currently 6-to-3 in favor of URL. Not necessarily a large number of votes yet though, if you would like to abstain it would be helpful indicate that so we can get an idea of participation.

@attack68
Copy link
Contributor

I would also vote for short. Probably # GHXXXX. I find the second # unnecessary.

agreed

@WillAyd
Copy link
Member

WillAyd commented Oct 15, 2023

No preference

@jorisvandenbossche
Copy link
Member

It might be an obvious advantage of URLs, but to explicitly state why I prefer URLs: it allows me to simply click on it in my IDE to go to the issue (no need to copy paste the number, and then manually construct the url adding the number)

And actually also for adding it in the code, I typically copy paste it from the issue web page, and then copying the whole url instead of only selecting the number of the url or from the title isn't any harder (or maybe even easier)

(and if we go for short, I would prefer GH-xxxx style)

@lukemanley
Copy link
Member

No preference, but +1 for consistency

@simonjayhawkins
Copy link
Member

It might be an obvious advantage of URLs, but to explicitly state why I prefer URLs: it allows me to simply click on it in my IDE to go to the issue

+1

@rhshadrach
Copy link
Member Author

This fell off my radar for a bit, but I'd like to pick it back up. We have 7 for URLs and 4 for a shortened form of some sort. If the short voters do not have objections, in a few weeks I'm going to start working on conforming the tests to URL - ideally with some form of pre-commit check.

cc @phofl @bashtage @attack68

@Dr-Irv
Copy link
Contributor

Dr-Irv commented Feb 12, 2024

Mild preference for URLs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Needs Discussion Requires discussion from core team before further action Testing pandas testing functions or related to the test suite
Projects
None yet
Development

No branches or pull requests