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

added option where to branch from (if different than the default branch) #107

Merged
merged 5 commits into from May 31, 2022

Conversation

FitzgeraldKrudde
Copy link

I implemented specifying where to branch from.
Useful when you want to branch from a different branch than the default branch.

I tested the change and verified the resulting PR in the Azure GUI. This looked OK to me.

Please review/QA/merge.

Also see: #49

Copy link
Collaborator

@operte operte left a comment

Choose a reason for hiding this comment

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

I left a couple of comments.

azdo allows picking the source and target branches. This PR assumes that these are always the same. I have two main questions:

  • If we want to continue with this approach, I think we should pick a better option name.
  • We could also allow the user to specify source and target branches separately, following the azdo cli more closely.

Not sure what the best option is. Let me know.

src/doing/pr/commands.py Show resolved Hide resolved
src/doing/pr/commands.py Outdated Show resolved Hide resolved
src/doing/pr/commands.py Outdated Show resolved Hide resolved
src/doing/pr/create_pr.py Show resolved Hide resolved
@FitzgeraldKrudde
Copy link
Author

Our usecase is that we want to branch from (and to) a different branch than master, for example develop.
I.e. source and target branch name are intentionally combined.

I would prefer than a better named parameter as opposed to using 2 parameters.
I think starting-branch sounds like a good name then.

Is this OK?

@operte
Copy link
Collaborator

operte commented May 30, 2022

Yes, I see. I also believe it's not common to branch from one branch to a different one.

I thought a bit more about it and I'm even less happy with the other names, so let's keep default-branch. But let's change the docs a bit. I'll update my comments.

src/doing/pr/create_pr.py Show resolved Hide resolved
src/doing/workon/commands.py Outdated Show resolved Hide resolved
src/doing/workon/commands.py Outdated Show resolved Hide resolved
src/doing/pr/create_pr.py Show resolved Hide resolved
docs/reference/manual/pr_create.md Show resolved Hide resolved
@operte
Copy link
Collaborator

operte commented May 30, 2022

Can you think of any unit tests that we could add to the project?

@FitzgeraldKrudde
Copy link
Author

I am a bit of a python newbie.. I am not familiar with python unit testing

docs/reference/manual/pr_create.md Outdated Show resolved Hide resolved
@operte
Copy link
Collaborator

operte commented May 31, 2022

Thanks! :)

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.

None yet

2 participants