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

cml-pr name branch #553

Merged
merged 5 commits into from
May 24, 2021
Merged

cml-pr name branch #553

merged 5 commits into from
May 24, 2021

Conversation

DavidGOrtega
Copy link
Contributor

GH gives branch names with all the ref /refs/heads/branch using branch name like this seems possible through their api but is breaking later on their UI

  • Extracts the last part of the branch name

closes #551

@DavidGOrtega DavidGOrtega temporarily deployed to test-internal May 21, 2021 08:12 Inactive
@DavidGOrtega DavidGOrtega marked this pull request as draft May 21, 2021 08:12
@DavidGOrtega DavidGOrtega marked this pull request as ready for review May 21, 2021 09:01
@0x2b3bfa0
Copy link
Member

TypeError: Cannot read property 'split' of undefined
    at branch_name (/usr/local/lib/node_modules/@dvcorg/cml/src/drivers/github.js:22:24)
    at Github.get branch [as branch] (/usr/local/lib/node_modules/@dvcorg/cml/src/drivers/github.js:298:12)
    at CML.branch (/usr/local/lib/node_modules/@dvcorg/cml/src/cml.js:88:13)
    at CML.pr_create (/usr/local/lib/node_modules/@dvcorg/cml/src/cml.js:267:31)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async run (/usr/local/lib/node_modules/@dvcorg/cml/bin/cml-pr.js:15:10)

Running locally in the absence of the GITHUB_REF environment variable. Should we cover this edge case?

@DavidGOrtega DavidGOrtega temporarily deployed to test-internal May 21, 2021 09:41 Inactive
@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented May 21, 2021

Base branches with slashes in their names, like experiment/break-everything won't work on GitHub and, even if they did, would produce inconsistent results with other drivers. Admittedly, I did it on purpose. 😈

GITHUB_REF=experiment/break-everything cml-pr ···
{
  status: 422,
  headers: {···},
  request: {
    method: 'POST',
    url: 'https://api.github.com/repos/···/···/pulls',
    headers: {···},
    body: '{"head":"break-everything-cml-pr-···","base":"break-everything","title":"CML PR for break-everything 1a6ba96f","body":"\\nAutomated commits for https://github.com/···/···/commit/··· created by CML.\\n  "}',
    request: {···}
  },
  errors: [ { resource: 'PullRequest', field: 'base', code: 'invalid' } ],
  documentation_url: 'https://docs.github.com/rest/reference/pulls#create-a-pull-request'
}

@DavidGOrtega DavidGOrtega temporarily deployed to test-internal May 21, 2021 10:01 Inactive
@DavidGOrtega DavidGOrtega temporarily deployed to test-internal May 21, 2021 10:04 Inactive
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Nevertheless, the original error still is a mystery.

@0x2b3bfa0 0x2b3bfa0 added bug Something isn't working cml-pr Subcommand labels May 21, 2021
@casperdcl
Copy link
Contributor

casperdcl commented May 21, 2021

Base branches with slashes in their names, like experiment/break-everything won't work on GitHub

They should? Sounds like a bug

@DavidGOrtega
Copy link
Contributor Author

@casperdcl @0x2b3bfa0 only happens apparently with refs/heads/

@0x2b3bfa0
Copy link
Member

They should? Sounds like a bug

They should. Period. 😉

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented May 21, 2021

only happens apparently with refs/heads

Apparently, because I can't reproduce it with a minimal example.

src/drivers/github.js Outdated Show resolved Hide resolved
Co-authored-by: Casper da Costa-Luis <work@cdcl.ml>
@DavidGOrtega DavidGOrtega temporarily deployed to test-internal May 24, 2021 16:58 Inactive
@casperdcl casperdcl closed this in 3cbbce0 May 24, 2021
@casperdcl casperdcl merged commit 3cbbce0 into master May 24, 2021
@casperdcl casperdcl deleted the cml-pr-name-fix-branch-name branch May 24, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cml-pr Subcommand
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cml-pr convention name seems to be breaking GH's branches page
3 participants