Skip to content

setup GitHub repo automatically#955

Merged
jph00 merged 1 commit into
masterfrom
auto-setup
Aug 29, 2022
Merged

setup GitHub repo automatically#955
jph00 merged 1 commit into
masterfrom
auto-setup

Conversation

@hamelsmu

@hamelsmu hamelsmu commented Aug 29, 2022

Copy link
Copy Markdown
Contributor

Enhancements Added

  1. Updates the URL and Description
  2. Enables GitHub Pages from the root of the gh-pages branch (some slighly hacky things are required to make this happen see below comments).

How this was tested

Creating a repo with the gh cli

  • gh repo create ntest --clone --public && cd ntest && nbdev_new
  • edit settings.ini and run nbdev_new again
  • delete settings.ini and run nbdev_new again

Creating a repo with the Github web interface

In each case, I verified that the description, url were updated and that pages are properly enabled.

cc: @jph00 @seeM

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Comment thread nbdev/cli.py
from ghapi.core import GhApi
api = GhApi(owner=cfg.user, repo=cfg.repo, token=token)
try: # repo needs something in it before you can enable pages
cmds = L(['git add settings.ini', "git commit -m'add settings'", 'git config push.default current', 'git push'])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When you are first initializing a github repo, it cannot be empty before you enable pages. It has to contain at least one file. I don't love executing git commands on behalf of the user. Another alternative would be to commit an empty file, but that clutters the repo IMO.

Curious on what your opinions are here. I put this in its own try block because I suspect something could go wrong here and I don't want to disrupt the onboarding process.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's fine - whether we exec git for the user vs use ghapi for the user seems much the same afaict.

FYI I don't think map here is useful -- seems a plain for loop would be simpler and just as concise. Generally map is most useful for the situation where you want to return a list.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At least until we release it, perhaps best not to have the try, so we can find out if there are errors, and if so, what errors come up?

Comment thread nbdev/cli.py
cmds.map(partial(run, ignore_ex=True))
api.enable_pages(branch='gh-pages')
except HTTPError: print("Could not enable GitHub Pages automatically.")
try: api.repos.update(homepage=f'{cfg.doc_host}/{cfg.doc_baseurl}', description=cfg.description)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We are able to update the homepage and description with ghapi!

Comment thread nbdev/cli.py
try: # repo needs something in it before you can enable pages
cmds = L(['git add settings.ini', "git commit -m'add settings'", 'git config push.default current', 'git push'])
cmds.map(partial(run, ignore_ex=True))
api.enable_pages(branch='gh-pages')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If the repo is empty, we get a 403 error: conflict if trying to enable pages. Depending on how you create your repo before initialiing nbdev this is problematic. This is why the git commit step exists above.

@hamelsmu hamelsmu requested review from jph00 and seeM and removed request for seeM August 29, 2022 18:29
@hamelsmu hamelsmu added the enhancement New feature or request label Aug 29, 2022
@hamelsmu hamelsmu changed the title setup repo automatically setup GitHub repo automatically Aug 29, 2022
@jph00 jph00 merged commit 9565f0f into master Aug 29, 2022
@jph00 jph00 deleted the auto-setup branch August 29, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants