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

ci: Copy properties from issue to pull request #1310

Merged
merged 15 commits into from
Nov 20, 2020

Conversation

piotradamczyk5
Copy link
Contributor

@piotradamczyk5 piotradamczyk5 commented Nov 10, 2020

Fixes #1092

Test Plan

How do we know the code works?

When creating pull request labels, assignees and estimates are copied from source issue.
Source issue is discovered by:

To test it locally use command:
flankScripts pullRequest copyProperties --github-token=<github token> --zenhub-token=<zenhub token, contact me if you would like to test it> --pr-number=<number of PR>

Checklist

  • Find source issue by description or branch name
  • Copy Labels
  • Copy assignees
  • Copy estimations
  • Attach to github action
  • Documented
  • Unit tested

@piotradamczyk5 piotradamczyk5 self-assigned this Nov 10, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Nov 10, 2020

Timestamp: 2020-11-20 15:12:37
Buildscan url for ubuntu-workflow run 374598479
https://gradle.com/s/cathvdosoytiw

@piotradamczyk5 piotradamczyk5 force-pushed the #1092_copy_properties_to_pull_request branch 2 times, most recently from 7409fbd to fe8d175 Compare November 10, 2020 17:24
@codecov-io
Copy link

codecov-io commented Nov 10, 2020

Codecov Report

Merging #1310 (cf593ce) into master (501ea71) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1310   +/-   ##
=========================================
  Coverage     78.99%   78.99%           
  Complexity      703      703           
=========================================
  Files           244      244           
  Lines          4685     4685           
  Branches        825      825           
=========================================
  Hits           3701     3701           
  Misses          555      555           
  Partials        429      429           

@bootstraponline
Copy link
Contributor

I think we always want a reference to the ticket in the description for pull requests related to an issue (this is how GitHub links PRs to issues). GitHub doesn't understand the branch convention.

@piotradamczyk5 piotradamczyk5 force-pushed the #1092_copy_properties_to_pull_request branch from 0bbe934 to 34d0d0b Compare November 12, 2020 13:28
@piotradamczyk5
Copy link
Contributor Author

I think we always want a reference to the ticket in the description for pull requests related to an issue (this Yis how GitHub links PRs to issues). GitHub doesn't understand the branch convention.

Yes, the branch is just a fallback :) 99% of cases it will take it from description

private suspend fun getLabelsFromIssue(githubToken: String, issueNumber: Int) =
Fuel.get("https://api.github.com/repos/Flank/flank/issues/$issueNumber/labels")
.appendHeader("Accept", "application/vnd.github.v3+json")
.appendHeader("Authorization", "token $githubToken")
Copy link
Contributor

Choose a reason for hiding this comment

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

In GitHubApi.kt we have function appendHeaders. This function adds these headers to the request. Maybe we could use these functions in this and other places? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that this is very good idea

@piotradamczyk5 piotradamczyk5 force-pushed the #1092_copy_properties_to_pull_request branch from 04f5aca to cc30c4e Compare November 17, 2020 14:40
suspend fun setAssigneesToPullRequest(githubToken: String, pullRequestNumber: Int, assignees: List<String>) {
Fuel.post("https://api.github.com/repos/Flank/flank/issues/$pullRequestNumber/assignees")
.appendHeader("Accept", "application/vnd.github.v3+json")
.appendHeader("Authorization", "token $githubToken")
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we can use .appendHeaders(githubToken) too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed!

Copy link
Contributor

@pawelpasterz pawelpasterz left a comment

Choose a reason for hiding this comment

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

I've created test issue #1330 and pr #1331 and ran script according to the test plan. It's amazing how fast it is, I really like this feature. The only difference is I am assigned in PR. Also, it could be useful to copy epics

@pawelpasterz
Copy link
Contributor

pawelpasterz commented Nov 18, 2020

Any idea why it was repeated 3 times?
Screenshot 2020-11-18 at 14 41 16

@piotradamczyk5
Copy link
Contributor Author

Any idea why it was repeated 3 times?
Screenshot 2020-11-18 at 14 41 16

Zen hub API set estimates each time you run a script where Github set diff only

@piotradamczyk5 piotradamczyk5 marked this pull request as draft November 18, 2020 14:29
@piotradamczyk5 piotradamczyk5 marked this pull request as ready for review November 18, 2020 15:04
@piotradamczyk5
Copy link
Contributor Author

piotradamczyk5 commented Nov 18, 2020

I've created test issue #1330 and pr #1331 and ran script according to the test plan. It's amazing how fast it is, I really like this feature. The only difference is I am assigned in PR. Also, it could be useful to copy epics

Fixed! Thanks for finding it!

In case of copy epics, it is not so easy using Zenhub API, because it does not return epic for an issue. You could just get all epics and list issues in it, so it will not be very optimal

@pawelpasterz
Copy link
Contributor

I've created test issue #1330 and pr #1331 and ran script according to the test plan. It's amazing how fast it is, I really like this feature. The only difference is I am assigned in PR. Also, it could be useful to copy epics

Fixed! Thanks for finding it!

In case of copy epics, it is not so easy using Zenhub API, because it does not return epic for an issue. You could just get all epics and list issues in it, so it will not be very optimal

I see, thanks for info.

url.endsWith("/releases/latest") && request.containsSuccessHeader() -> request.buildResponse(
GitHubRelease("v20.08.0").toJson(),
200
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use one style of argument formatting in this file?

Copy link
Contributor

@adamfilipow92 adamfilipow92 left a comment

Choose a reason for hiding this comment

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

👍

@piotradamczyk5 piotradamczyk5 force-pushed the #1092_copy_properties_to_pull_request branch from a3753e9 to bd87d68 Compare November 20, 2020 15:07
@piotradamczyk5 piotradamczyk5 merged commit ac5a2a8 into master Nov 20, 2020
@piotradamczyk5 piotradamczyk5 deleted the #1092_copy_properties_to_pull_request branch November 20, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitHub action to validate is any user is assigned to pull request
5 participants