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

[github.d] don't mention bugzilla for phobos PRs #305

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thewilsonator
Copy link
Contributor

No description provided.

@@ -51,6 +51,11 @@ string markdownEscape(string desc)
return app.data;
}

bool usesGitHubIssues(in ref PullRequest pr)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

normally I would do usesBugzillaIssues but I don't know what the full set of repos that use it are.

Copy link
Member

Choose a reason for hiding this comment

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

Were you looking for this?

// D projects which use Bugzilla for bug tracking.
static immutable bugzillaProjectSlugs = ["dlang/dmd", "dlang/druntime", "dlang/phobos",
"dlang/dlang.org", "dlang/tools", "dlang/installer"];

@CyberShadow
Copy link
Member

This doesn't look like the right fix. Wouldn't we want to simply encourage people to create GitHub issues instead of Bugzilla issues for these repositories?

We do want to keep the policy that a PR references an issue, right? Otherwise that pulls in questions such as what to list in the changelog.

@jmdavis
Copy link
Member

jmdavis commented Dec 6, 2024

This doesn't look like the right fix. Wouldn't we want to simply encourage people to create GitHub issues instead of Bugzilla issues for these repositories?

I would expect that the move to github would just change which issues we're referencing. So, instead of something like fix bugzilla issue 42, it would be fix github issue 42, and down the line, once we're no longer using bugzilla for issues at all, we could consider changing it to fix issue 42 like it was before. If we can't have the bot handle github issues in the same fashion that it's been handling bugzilla issues, then that's a definite downgrade.

@CyberShadow
Copy link
Member

We technically don't need the bot to support GitHub issues, since GitHub takes care of the duties that the bot performed on Bugzilla (linking PRs in issues when one is created to address it, and closing issues that fix PRs). I think the only thing that may need changing is the warning that is shown if a PR doesn't reference any issues at all.

@jmdavis
Copy link
Member

jmdavis commented Dec 6, 2024

We technically don't need the bot to support GitHub issues, since GitHub takes care of the duties that the bot performed on Bugzilla (linking PRs in issues when one is created to address it, and closing issues that fix PRs). I think the only thing that may need changing is the warning that is shown if a PR doesn't reference any issues at all.

Well, if github manages it automatically somehow, then great. But presumably we then need the instructions on PRs to tell you whatever it is that you need to put into the commit message instead of what we've been doing so that github will pick it up properly in addition to not mentioning bugzilla any longer. And either way, we'll likely need to change the changelog generator so that it understands the change and provides links to github where appropriate instead of bugzilla (though that's obviously separate from these changes).

@@ -51,6 +51,11 @@ string markdownEscape(string desc)
return app.data;
}

bool usesGitHubIssues(in ref PullRequest pr)
{
return pr.repoSlug.among("dlang/phobos");
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this needs to be compared against 0, since among returns 0 if it's not found and a one-based index of which one was found if one is found.

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.

3 participants