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

Add GitHub Actions CI support. #64

Merged
merged 1 commit into from
Oct 3, 2019
Merged

Add GitHub Actions CI support. #64

merged 1 commit into from
Oct 3, 2019

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Oct 2, 2019

  1. this requires Update all dependencies #61 for the lock file addition. If you guys don't want a lockfile, I can change this to just use npm i
  2. I don't know what exactly is required for tests to run. A token?
  3. I also plan to add Windows testing later -- DONE, but I might need to drop it from here and tackle it later separately

package.json Outdated Show resolved Hide resolved
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

Alright, let's focus on getting this up and running.

I untied this patch from #61. I have added Ubuntu only and I'm using npm install for starters.

@XhmikosR XhmikosR mentioned this pull request Oct 3, 2019
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

If someone could add the needed secrets on the repo secrets page, let me know what names you used and I'll adapt the patch so that it uses them when running tests.

@rvagg
Copy link
Member

rvagg commented Oct 3, 2019

according to https://help.github.com/en/articles/virtual-environments-for-github-actions#github_token-secret we might be able to make use of GITHUB_TOKEN. You'll need to do this in the action:

    env:
      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Then if you can run some bash before running the tests, try this (for Linux):

mkdir -p .config/changelog-maker/ && echo '{"user":"nodejs","token":"'${GITHUB_TOKEN}'"}' > .config/changelog-maker/config.json

The awkward bit is the "user", I'm not sure whether it matters. It gets combined with the token for the Authorization: Basic <base64 of user:token> header (see https://github.com/rvagg/ghutils/blob/master/ghutils.js#L13). I don't see what user GITHUB_TOKEN is associated with or even whether it matters.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

Alright, GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" doesn't seem to work.

We need to add and pass the exact variables including the user.

@rvagg
Copy link
Member

rvagg commented Oct 3, 2019

if that doesn't work, the alternatives i see are: (1) generate a token for an account owned by the org or (2) modify ghutils to directly use the Authorization: token <token> header. I'm unclear about whether this will break existing use of ghutils but we could probably find out.

@rvagg
Copy link
Member

rvagg commented Oct 3, 2019

@XhmikosR you need to do something with GITHUB_TOKEN, see the bash I put in that previous comment to put it into a config file that's picked up by ghauth.

@rvagg
Copy link
Member

rvagg commented Oct 3, 2019

       - run: mkdir -p .config/changelog-maker/ && echo '{"user":"nodejs","token":"'${GITHUB_TOKEN}'"}' > .config/changelog-maker/config.json
       - run: npm test

?

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

@rvagg does ghauth pick up user variables about user and token or does it need a config explicitly?

I will try this later, but I'm on Windows currently.

@rvagg
Copy link
Member

rvagg commented Oct 3, 2019

@XhmikosR it picks it up from a config file that looks like {"user":"foo","token":"bar"}. It should be created on Windows @ %LOCALAPPDATA%/changelog-maker/config.json (as per https://github.com/LinusU/node-application-config#config-location). When you first invoke ghauth (see collect-commit-labels.js) and you don't have this file, it should prompt you on the commandline to enter github credentials and it'll make a token for you and save it to that file for future use.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

@rvagg so as things are right now, we need tho create these configs. Would it be possible to make ghauth use env vars too? I think it would simplify things a lot.

Regardless, I'll experiment with this and let you know how it goes.

@rvagg
Copy link
Member

rvagg commented Oct 3, 2019

Would it be possible to make ghauth use env vars too

yes, it would certainly simplify the CI case .. I'm not sole maintainer so any such PR will need discussion.

@XhmikosR XhmikosR force-pushed the gh-actions branch 4 times, most recently from 08185bf to b3e6545 Compare October 3, 2019 10:52
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

It works! The path is ~/.config/changelog-maker/config.json.

@XhmikosR XhmikosR marked this pull request as ready for review October 3, 2019 10:58
@XhmikosR XhmikosR requested a review from rvagg October 3, 2019 10:59
@XhmikosR XhmikosR force-pushed the gh-actions branch 2 times, most recently from 9adf48b to f8423b7 Compare October 3, 2019 11:00
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

Only thing I don't know is if this will work for forks. I guess we'll find out later.

/CC @targos for a quick review too :)

@XhmikosR XhmikosR requested a review from targos October 3, 2019 11:01
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 3, 2019

I added Windows too. I don't think the shell used on Windows demonstrates the failures I had locally, though, because CI is passing even with the old gitexec.

That being said, this should better than nothing.

Copy link
Member

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

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

Nice!

@XhmikosR XhmikosR merged commit 90ae723 into master Oct 3, 2019
@XhmikosR XhmikosR deleted the gh-actions branch October 3, 2019 12:23
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.

3 participants