Skip to content

Feature/create repo with gitignore license#3746

Merged
vilmibm merged 12 commits intocli:trunkfrom
g14a:feature/repo-with-gitignore-license
Jun 28, 2021
Merged

Feature/create repo with gitignore license#3746
vilmibm merged 12 commits intocli:trunkfrom
g14a:feature/repo-with-gitignore-license

Conversation

@g14a
Copy link
Contributor

@g14a g14a commented May 30, 2021

This commit adds a feature where you can initialize a repository with a .gitignore and/or a license template.

Fixes #1907

Attaching screenshots for reference:

  • image
  • image
  • image
  • image

@vilmibm
Copy link
Contributor

vilmibm commented Jun 1, 2021

Thanks!

@ampinsk are you cool with the final design approach here? if so, I'll review the code ✨

@vilmibm vilmibm requested review from ampinsk and vilmibm June 1, 2021 20:40
@g14a
Copy link
Contributor Author

g14a commented Jun 9, 2021

Hey @ampinsk, just wanted to get a follow up regarding the design flow here :)

@ampinsk
Copy link

ampinsk commented Jun 10, 2021

Hey sorry for the delay! This approach makes sense to me 👍

@g14a
Copy link
Contributor Author

g14a commented Jun 14, 2021

@vilmibm since it is approved by @ampinsk I think we're good to go :)

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

oops, meant to comment instead of comment-review.

I'll do a complete review later today or tomorrow.

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

Thank you for taking this on! Unfortunately, as it stands, this PR presents too great a risk for our users. This was discussed in the original issue; consider these two scenarios:

scenario 1: empty local repo

  • mkdir foo; cd foo; git init
  • gh repo create # adding a license or ignore
  • git pull origin main

This puts you in a happy state where your local branch matches the remote and you are free to add commits.

scenario 2: non-empty local repo

  • mkdir foo; cd foo; git init
  • echo "foo" > foo.txt; git add foo.txt; git commit -m'foo'
  • gh repo create # adding license/ignore
  • git pull origin main # results in fatal: refusing to merge unrelated histories
  • git push origin main # results in rejected push

This puts you in a broken state that requires git knowledge to correct.


Scenario 2 is totally unacceptable from our perspective; many users will be confused and frustrated by this and it's very easy to make it happen. Without a remediation plan for scenario 2, this PR won't be able to move forward.

Unfortunately, I can't think of any strategies around this given what tools the API currently exposes. The safest thing is to ensure that changes like this -- adding an ignore or a license -- start as a local commit that get pushed. However, that would require duplicating .com functionality (ie actually getting templates and rendering them) which is non-trivial effort and may end up being brittle.

One path forward here might be only exposing gitignore/license generation for the gh repo create REPONAME case. Right now we treat that as a separate code path (for historical reasons I can't entirely remember) that is still interactive but has different questions. In that case, we're creating a repo from scratch and then optionally cloning it: It's entirely safe to add ignore/license in that mode. I'm open to this PR being retooled for that.

@g14a
Copy link
Contributor Author

g14a commented Jun 16, 2021

This puts you in a broken state that requires git knowledge to correct.

oh wow thank you for the very detailed explanation @vilmibm I hadn't tried the 2nd scenario while I was developing this.

Dahalan43
Dahalan43 approved these changes Jun 16, 2021
@g14a
Copy link
Contributor Author

g14a commented Jun 17, 2021

@vilmibm I've made the changes and added some validations and tests too. Would like some feedback :)

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

oof, this is not on you, but I realized in testing that gh repo create foo needs some additional tweaking to make this work.

Currently, it creates the repo on GitHub and then creates a new local repo, but it doesn't yet know what the default branch and can't actually mirror the content locally.

The core issue here is that there is some lag between creating the repo on GitHub and it actually being clone'able.

If you're up for it, I think the next step would be experimenting with a for loop with an interval and timeout for blocking the command until the repo exists then doing a clone (instead of localInit).

@g14a
Copy link
Contributor Author

g14a commented Jun 23, 2021

The core issue here is that there is some lag between creating the repo on GitHub and it actually being clone'able.

@vilmibm I've pushed the changes and tested it on my local as well. Ten out of ten times, I don't experience any lag which you were referring to. Could you elaborate on how I can reproduce it?

I even tried scripting it with gh repo create random --gitignore Go --license mit --public --confirm and it clones perfectly.

@vilmibm
Copy link
Contributor

vilmibm commented Jun 28, 2021

huh, ok! I just assumed there was lag because of how we engineered this -- I didn't actually check. but I also cannot produce a failure with this code (which, btw, I really enjoy the experience of). I'll do a final review.

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

Nice work :D

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.

Ability to initialize repo with .gitignore and license

4 participants