Allow deploy keys as alternative to access tokens

ponylang / library-documentation-action

:horse: Generates documentation for Pony libraries

BSD 2-Clause "Simplified" License

4 stars 3 forks source link

Allow deploy keys as alternative to access tokens #13

Closed Trundle closed 3 years ago

Trundle commented 3 years ago

This pull request makes the action also work with deploy keys instead of access tokens. Compared to a personal access token, deploy keys have the advantage that they can be limited to a single repository.

SeanTAllen commented 3 years ago

@Trundle- you can use make pylint to run the linting that will happen at CI time locally. As long as you have docker installed it will work.

SeanTAllen commented 3 years ago

@Trundle how much of this have you tested and how?

Normally I test in the action-testing repo and verify everything works (but you arent a contributor to that- so I could test it all there).

It took a while to get the "retry on failure" code working so I'm wary of changes to it.

Trundle commented 3 years ago

I tested it in https://github.com/Trundle/pony-templates, pointing to my fork of the action: https://github.com/Trundle/pony-templates/runs/2171361022?check_suite_focus=true

I only tested with a deploy key though.

SeanTAllen commented 3 years ago

@EpicEric can you give this a review as well?

SeanTAllen commented 3 years ago

@Trundle I'll give it a go in the action-testing as well now.

SeanTAllen commented 3 years ago

I tested with existing config in action-testing and it all worked, so the happy path is good. Still a little wary of the change in the git push failure. I'd like @EpicEric to review as he is better with python than I am.

But it looks pretty good other than a few things here and there. Thanks @Trundle. Ping me when you've made some of the requested updates.

ponylang-main commented 3 years ago

Hi @Trundle,

The changelog - added label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 13.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

SeanTAllen commented 3 years ago

@Trundle is this ready to squash and merge?

Trundle commented 3 years ago

So it stopped working in my repository now, but I don't see why yet :raised_eyebrow: I think I messed up my keys. I'll have a look tomorrow.

SeanTAllen commented 3 years ago

@Trundle ok. let me know if I should re-test any functionality.

How did you determine that it stopped working? Did you do a run and it failed at some point?

Trundle commented 3 years ago

Exactly. I triggered a run (https://github.com/Trundle/pony-templates/runs/2180017638?check_suite_focus=true) and it fails when fetching and complains about SSH keys. I deleted my previous deploy keys, hence I think this is rather some mistake in my setup, but I'm too tired right now to think it through.

Trundle commented 3 years ago

I learned that OpenSSH requires keys to end with a newline. I now added an explicit check whether the passed key ends in a newline, because I expect that to be a common c/p mistake with GitHub secrets.

There was also a refactoring glitch when I changed back to os.system, fixed with 2721b94bbde3d97e685e2218d8bef904204a6745. I think this is good to squash and merge now!

SeanTAllen commented 3 years ago

Thanks @Trundle. I'll give it another quick test and then... Away we go!