Activity

1. hugovk

   changed the title [-]bedevere could add docs label for prs, that touch Doc/tools/.nitignore[/-] [+]Add docs label for PRs that touch Doc/tools/.nitignore[/+] on Jan 19, 2024
2. hugovk commented on Jan 19, 2024

   

   hugovk

   on Jan 19, 2024

   Member

   More actions

   Good idea!

   I found it a bit annoying the label is added for PRs touching rst files in Docs/, but not those touching rst plus .nitignore.

   Reacted by Alex Waygood
3. skirpichev commented on Jan 19, 2024

   

   skirpichev

   on Jan 19, 2024

   MemberAuthor

   More actions

   PR: #617

   Reacted by Hugo van Kemenade
4. hugovk

   added

   enhancement

   on Jan 19, 2024
5. ezio-melotti commented on Jan 20, 2024

   

   ezio-melotti

   on Jan 20, 2024

   Member

   More actions

   I found it a bit annoying the label is added for PRs touching rst files in Docs/, but not those touching rst plus .nitignore.

   Why does this happen? Isn't bedevere already adding the docs label whenever at least an .rst file has been changed, regardless of the other files?
6. skirpichev commented on Jan 20, 2024

   

   skirpichev

   on Jan 20, 2024

   MemberAuthor

   More actions

   Looking in the sources, it doesn't add any labels (pr_labels == []) if there is some file, which doesn't match known patterns:
   

   bedevere/bedevere/prtype.py

   Lines 42 to 51 in b5bcd24

   	for filename in filenames:
   	if util.is_news_dir(filename):
   	news = True
   	filepath = pathlib.PurePath(filename)
   	if filepath.suffix == ".rst":
   	docs = True
   	elif filepath.name.startswith("test_"):
   	tests = True
   	else:
   	return pr_labels

   
   Another case: #605
7. ezio-melotti commented on Jan 20, 2024

   

   ezio-melotti

   on Jan 20, 2024

   Member

   More actions

   So the problem is the return (prematurely?) terminating the loop? Is that intentional?
8. skirpichev commented on Jan 20, 2024

   

   skirpichev

   on Jan 20, 2024

   MemberAuthor

   More actions

   Is that intentional?

   I guess - so. Or every pr that has some *.rst file could skip news.
9. AlexWaygood commented on Jan 20, 2024

   

   AlexWaygood

   on Jan 20, 2024

   Member

   More actions

   Is that intentional?

   I guess - so. Or every pr that has some *.rst file could skip news.

   That wouldn't be correct — there are lots of PRs that introduce new features and add docs for the new feature as part of the same PR. Most of those PRs need news entries.
10. skirpichev commented on Jan 20, 2024

    

    skirpichev

    on Jan 20, 2024

    MemberAuthor

    More actions

    (On another hand, most PRs, that touch only test files - probably could skip news;) That's another story, however.)

    Reacted by Alex Waygood
11. ezio-melotti commented on Jan 20, 2024

    

    ezio-melotti

    on Jan 20, 2024

    Member

    More actions

    Ok, so the goal is to add the docs label only to PRs that are "docs-only", i.e. they only change documentation and possibly files related to the documentation (like .nitignore). If a PR adds a new feature and edits the docs to document it, it's not "docs-only" and shouldn't get the docs label.

    (Maybe this should be spelled out clearly in a comment/docstring, including the expected behavior for other labels, since the intention is currently not too clear IMHO).

    Reacted by Alex Waygood
12. AA-Turner

    closed this as completedin #617on Jan 20, 2024