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

Generates Bitbucket Data Center PR URL and retrieves repoId for cross-forks #4184

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sergeibbb
Copy link
Member

@sergeibbb sergeibbb commented Mar 28, 2025

Description

relates to #4142

Checklist

  • I have followed the guidelines in the Contributing document
  • My changes follow the coding style of this project
  • My changes build without any errors or warnings
  • My changes have been formatted and linted
  • My changes include any required corresponding changes to the documentation (including CHANGELOG.md and README.md)
  • My changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • My changes have a descriptive commit message with a short title, including a Fixes $XXX - or Closes #XXX - prefix to auto-close the issue that your PR addresses

@sergeibbb sergeibbb linked an issue Mar 28, 2025 that may be closed by this pull request
8 tasks
@sergeibbb sergeibbb force-pushed the bug/create-prs-bitbucket-dc branch from df15357 to 2fea345 Compare March 28, 2025 17:04
@eamodio
Copy link
Member

eamodio commented Apr 8, 2025

@sergeibbb can you rebase this?

@sergeibbb sergeibbb force-pushed the bug/create-prs-bitbucket-dc branch from 2fea345 to 9872ed6 Compare April 8, 2025 18:00
@sergeibbb
Copy link
Member Author

@eamodio

can you rebase this?

done

Copy link
Member

@eamodio eamodio left a comment

Choose a reason for hiding this comment

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

Couple of questions

Comment on lines 190 to 195
return this.splitArgPath(this.path);
}

protected splitArgPath(argPath: string): [string, string] {
const index = argPath.indexOf('/');
return [argPath.substring(0, index), argPath.substring(index + 1)];
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a new splitArgPath function, what do you think of just making splitPath take a path and change current callers to pass in this.path? That would give the callers more control.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I like it!

Copy link
Member Author

Choose a reason for hiding this comment

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

@eamodio : fixed

@sergeibbb sergeibbb force-pushed the bug/create-prs-bitbucket-dc branch from 9872ed6 to 526fe90 Compare April 9, 2025 12:57
@sergeibbb sergeibbb requested a review from eamodio April 9, 2025 12:58
Copy link
Member

@eamodio eamodio left a comment

Choose a reason for hiding this comment

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

:shipit:

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.

Create Pull Request action fails for remotes other than GitHub
2 participants