search

owid / etl

A compute graph for loading and transforming OWID's data
https://docs.owid.io/projects/etl
MIT License
58 stars 18 forks source link

✨ Improve `etl d draft-pr` tool #2777

Closed pabloarosado closed 3 weeks ago

pabloarosado commented 3 weeks ago

Following this discussion, now you don't need to write --new-branch and you can simply pass the name of the new branch as an argument.

I haven't implemented the --ready-to-review idea, because we don't want to create a PR that is ready for review with a draft title. If we do that, then we also need an argument for the title. At this point, I think it's just easier and more transparent to do it from github. It feels safer if the tool just drafts things. What do you think?

lucasrodes commented 3 weeks ago

Makes sense to not have the --ready-to-review for now. What is your opinion on replacing draft-pr to pr, though? No strong opinion on my end, just looking for short commands :p

lucasrodes commented 3 weeks ago

Couldn't find the behaviour for when no argument for new-branch is passed. Do you need to set the variable to the current branch?

pabloarosado commented 3 weeks ago

Thanks @lucasrodes, I think etl pr may be "too short" (it could be typed by accident). Let's leave it at etl draft-pr for now.

pabloarosado commented 3 weeks ago

Couldn't find the behaviour for when no argument for new-branch is passed. Do you need to set the variable to the current branch?

That is already implemented, when new_branch is None, in line 81. Isn't that what you meant?

lucasrodes commented 3 weeks ago

I think etl pr may be "too short" (it could be typed by accident). Let's leave it at etl draft-pr for now.

Alright! (not that it matters, but I meant etl d pr)

lucasrodes commented 3 weeks ago

That is already implemented, when new_branch is None, in line 81. Isn't that what you meant?

Got it ー didn't see it in the last changes and was confused. Makes sense, thanks!