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

setup GitHub repo automatically #955

Merged
merged 1 commit into from
Aug 29, 2022
Merged

setup GitHub repo automatically #955

merged 1 commit into from
Aug 29, 2022

Conversation

hamelsmu
Copy link
Contributor

@hamelsmu hamelsmu commented Aug 29, 2022

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

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

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
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
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
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?

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
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!

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
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 seeM and jph00 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