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

changelog github integration #466

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

burner
Copy link
Member

@burner burner commented Dec 17, 2023

graphql query works mostly

bearer not working anymore

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @burner!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + tools#466"

Copy link
Member

@ibuclaw ibuclaw left a comment

Choose a reason for hiding this comment

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

Let me know when it's no longer a draft.

changed.d Outdated Show resolved Hide resolved
@burner burner changed the title DRAFT: changelog github integration changelog github integration Feb 21, 2024
@burner
Copy link
Member Author

burner commented Feb 21, 2024

@ibuclaw I've taken another look, and I think there is nothing more to do until it fails in prod

@ibuclaw
Copy link
Member

ibuclaw commented Jan 14, 2025

FYI, you could perhaps test this with something like:

rdmd -version=Contributors_Lib changed v2.109.1..upstream/stable \
    --version=2.110.0 --date='Feb 01, 2025' --output=../dlang.org/changelog/2.110.0_pre.dd

Which is what's ran to generate the dlang.org changelog.

changed.d Outdated
}
if (revRange.length)
{
githubChanges.writeBugzillaChanges(w);
Copy link
Member

Choose a reason for hiding this comment

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

I think you should distinguish between Bugzilla and GitHub changes here with a $(GITHUB ####) macro, otherwise Github issue numbers will get Bugzilla links.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I fixed this!?

Comment on lines +358 to +360
string req = ("https://api.github.com/repos/%s/%s/issues?per_page=100"
~"&state=closed&since=%s&page=%s")
.format(project, repo, startDate.toISOExtString() ~ "Z", page);
Copy link
Member

Choose a reason for hiding this comment

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

https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28

Note

GitHub's REST API considers every pull request an issue, but not every issue is a pull request. For this reason, "Issues" endpoints may return both issues and pull requests in the response. You can identify pull requests by the pull_request key. Be aware that the id of a pull request returned from "Issues" endpoints will be an issue id. To find out the pull request id, use the "List pull requests" endpoint.

Definitely don't want every single pull request dumped into the changelog.

Otherwise we'll end up with...

+$(LI $(BUGZILLA 16483): move Foff to CGstate)
+$(LI $(BUGZILLA 16484): move dfoidx to CGstate)
+$(LI $(BUGZILLA 16485): move CSoff to CGstate)
+$(LI $(BUGZILLA 16486): move allregs to CGstate)
+$(LI $(BUGZILLA 16487): move NDPoff to CGstate)
+$(LI $(BUGZILLA 16488): move mfuncreg to CGstate)
+$(LI $(BUGZILLA 16489): move retsym to CGstate)
+$(LI $(BUGZILLA 16490): move pushoff to CGstate)
+$(LI $(BUGZILLA 16491): move msavereg to CGstate)
+$(LI $(BUGZILLA 16492): move stackpush to CGstate)
+$(LI $(BUGZILLA 16493): move BPoff to CGstate)
+$(LI $(BUGZILLA 16494): move pass to CGstate)
+$(LI $(BUGZILLA 16495): move stackchanged to CGstate)
+$(LI $(BUGZILLA 16496): move calledFinally to CGstate)
+$(LI $(BUGZILLA 16497): move regcon to CGstate)
+$(LI $(BUGZILLA 16498): move ESPtoEBP to CGstate $(LPAREN)#16497$(RPAREN))
+$(LI $(BUGZILLA 16499): move usednteh to CGstate)
+$(LI $(BUGZILLA 16500): move refparam to CGstate)
+$(LI $(BUGZILLA 16501): untangle some of the backend imports)
+$(LI $(BUGZILLA 16502): simplify union evc)
+$(LI $(BUGZILLA 16503): get rid of code_last$(LPAREN)$(RPAREN))
+$(LI $(BUGZILLA 16504): move regsave to CGstate)
+$(LI $(BUGZILLA 16505): move gotref to CGstate)
+$(LI $(BUGZILLA 16506): move reflocal to CGstate)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this looks at closed issues not pull requests. Am I missing something

Copy link
Member

Choose a reason for hiding this comment

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

It returns both. Otherwise I wouldn't be seeing a huge list of PRs in the generated changelog.

graphql query works mostly

bearer not working anymore
next thing to do is wire in the closed github issues even if there where
not closed by a commit
now I need to integrate the github issues into the output
@burner burner force-pushed the github_issue_changelog_integeration branch from 513d070 to 00bf95f Compare January 17, 2025 16:24
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