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

Fix branch name #649

Merged

Conversation

daddykotex
Copy link
Contributor

During merge/rebase, I may have accidentally changed the
branch name used when creating the MR. On Gitlab, we need
update/dependency-1.2.0 where as on GitHub we need:
owner:update/dependency-1.2.0.

@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #649 into topic/gitlab will decrease coverage by 0.46%.
The diff coverage is 40%.

Impacted file tree graph

@@               Coverage Diff                @@
##           topic/gitlab     #649      +/-   ##
================================================
- Coverage          57.9%   57.44%   -0.47%     
================================================
  Files                73       71       -2     
  Lines               974      954      -20     
  Branches             21       26       +5     
================================================
- Hits                564      548      -16     
+ Misses              410      406       -4
Impacted Files Coverage Δ
...la/org/scalasteward/core/application/Context.scala 0% <ø> (ø) ⬆️
...calasteward/core/vcs/data/NewPullRequestData.scala 84.61% <ø> (-1.1%) ⬇️
...ala/org/scalasteward/core/nurture/NurtureAlg.scala 0% <0%> (ø) ⬆️
...scala/org/scalasteward/core/vcs/VCSSelection.scala 0% <0%> (ø) ⬆️
...main/scala/org/scalasteward/core/vcs/package.scala 100% <100%> (ø) ⬆️
...ain/scala/org/scalasteward/core/util/package.scala 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20fae69...13b3ac9. Read the comment docs.

During merge/rebase, I may have accidentally changed the
branch name used when creating the MR. On Gitlab, we need
`update/dependency-1.2.0` where as on GitHub we need:
`owner:update/dependency-1.2.0`.
@daddykotex
Copy link
Contributor Author

Removed the whole VCSSpecifics thing

@fthomas fthomas merged commit 2543d2a into scala-steward-org:topic/gitlab Jul 3, 2019
rtyley added a commit to rtyley/scala-steward that referenced this pull request Feb 29, 2024
… Pull Requests

As described in scala-steward-org#3300, Scala Steward has been using a now-invalid form of `head`
query parameter when querying for existing GitHub Pull Requests. This has meant that
it's been trying to raise new pull requests when existing ones with the same branch
name were already present, leading to exceptions from the GitHub API.

The `head` parameter being used would look like:

```
[organization]/[repo]:[ref-name]
```

...however the documentation for the `List pull requests` endpoint says it should be
just:

```
[organization]:[ref-name]
```

(see https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests)

...it actually _does_ make sense to _not_ have the repo name in there, as it _is_
redundant - the path in the REST API request already contains /repos/{owner}/{repo}/pulls,
giving the original repo, and GitHub only allows one fork of a repo per organisation or user,
so including the user or org in the prefix should be sufficient to uniquely identify a fork.

Although the `[organization]/[repo]:[ref-name]` format must have worked in the past, it
looks like GitHub must have made a change that means if you use it to search for PRs by
branch name _now_, you'll get zero results - the PRs we're interested in won't come back.

Consequently, this change updates Scala Steward to use the approved `head` parameter format,
dropping the `/[repo]` part. The code in Scala Steward that constructed the parameter _was_
in the `forge.listingBranch()` method, but once it was fixed to remove `/[repo]`, it became
identical to the `forge.createBranch()` method, and so it made sense to go from having 2
methods back to just 1 (there was originally just one `headFor` method back before PR scala-steward-org#649).
mzuehlke pushed a commit that referenced this pull request Feb 29, 2024
As described in #3300, Scala Steward has been using a now-invalid form of `head`
query parameter when querying for existing GitHub Pull Requests. This has meant that
it's been trying to raise new pull requests when existing ones with the same branch
name were already present, leading to exceptions from the GitHub API.

The `head` parameter being used would look like:

```
[organization]/[repo]:[ref-name]
```

...however the documentation for the `List pull requests` endpoint says it should be
just:

```
[organization]:[ref-name]
```

(see https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests)

...it actually _does_ make sense to _not_ have the repo name in there, as it _is_
redundant - the path in the REST API request already contains /repos/{owner}/{repo}/pulls,
giving the original repo, and GitHub only allows one fork of a repo per organisation or user,
so including the user or org in the prefix should be sufficient to uniquely identify a fork.

Although the `[organization]/[repo]:[ref-name]` format must have worked in the past, it
looks like GitHub must have made a change that means if you use it to search for PRs by
branch name _now_, you'll get zero results - the PRs we're interested in won't come back.

Consequently, this change updates Scala Steward to use the approved `head` parameter format,
dropping the `/[repo]` part. The code in Scala Steward that constructed the parameter _was_
in the `forge.listingBranch()` method, but once it was fixed to remove `/[repo]`, it became
identical to the `forge.createBranch()` method, and so it made sense to go from having 2
methods back to just 1 (there was originally just one `headFor` method back before PR #649).
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.

2 participants