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

Configure Auto #74

Merged
merged 15 commits into from
Oct 19, 2020
Merged

Configure Auto #74

merged 15 commits into from
Oct 19, 2020

Conversation

tylerkrupicka
Copy link
Contributor

@tylerkrupicka tylerkrupicka commented Oct 6, 2020

What is changing

I added the basic configuration for auto, with the following things configured:

  • NPM releases (Release on Master merge, Canary versions)
  • Released comments (when a PR is released, comment on the PR)
  • All contributors (Automatically update all contributors when someone has a PR merged)

This PR will not work without some work from the person who runs the CircleCI builds, I'll try to document what you need to do below.

  • You'll need to set up an initial tag and release. Looks like the current version is 1.0.2.
  • You'll need to set up environment variables. Auto will need a token to publish to NPM, and a token to write release notes to GitHub.
  • You'll need to add labels for patch, minor, major, documentation, and skip-release, so it's easy to label PRs.

What else might be impacted?

Mainly your build process. I re-worked it a bit to include auto in the pipeline and have more discrete steps. It's not going to work until we make some minor changes to the builds. Let me know if you need any help setting that up or testing!

Which issue does this PR relate to?

Closes #24
Closes #11

Checklist

[ ] Tests are written and maintain or improve code coverage
[ ] I've tested this in my application using yarn link (if applicable)
[ ] You consent to and are confident this change can be released with no regression

@tylerkrupicka
Copy link
Contributor Author

I'm getting a circle coveralls error, but I didn't change that. Regardless this PR won't work without some Circle changes, so we can see if it works after updating the other things.

@reubenae
Copy link
Contributor

reubenae commented Oct 6, 2020

Thanks @tylerkrupicka ! Really appreciate this. I've added the labels and will look into the CircleCI changes unless @skodamarthi gets there before me. Will check in with you if or when I get stuck.

@skodamarthi
Copy link
Contributor

@tylerkrupicka Thanks for taking this up! I have done the first step of setting up initial tag and release for our latest version v1.0.3. About the 2nd step about environment variables, do we need to set this on local env file or can we try setting the environment variable on Circle CI and verify directly?

.circleci/config.yml Outdated Show resolved Hide resolved
@tylerkrupicka
Copy link
Contributor Author

You'll need to set up the environment variables on CircleCI directly, the env file is usally just for local development and you don't commit it. You'll need to add the NPM token you use for publishing the package, and a GitHub token that has rights to the repo.

@skodamarthi
Copy link
Contributor

skodamarthi commented Oct 6, 2020

You'll need to set up the environment variables on CircleCI directly, the env file is usally just for local development and you don't commit it. You'll need to add the NPM token you use for publishing the package, and a GitHub token that has rights to the repo.

@tylerkrupicka So I need to create the following two environment variables to CircleCI right? GH_TOKEN and NPM_TOKEN
For NPM_TOKEN, I have generated a new one of type "Automation" to work with CI/CD workflows. Is that ok? Or do I need to generate "Publish" type?
Screen Shot 2020-10-07 at 00 08 31

@skodamarthi
Copy link
Contributor

I'm getting a circle coveralls error, but I didn't change that. Regardless this PR won't work without some Circle changes, so we can see if it works after updating the other things.

Not sure why coveralls failed with that error, but saw on some circleci discussion that the error should usually get fixed by itself if you submit another commit. https://discuss.circleci.com/t/admin-must-opt-in-to-3rd-part-orbs-but-i-already-have/35552. Let's see if it works after you make a new commit for removing timezone.

@tylerkrupicka
Copy link
Contributor Author

@skodamarthi yeah an automation token should be what you want! I can make another commit to see if it works.

Thanks!

@skodamarthi
Copy link
Contributor

@skodamarthi yeah an automation token should be what you want! I can make another commit to see if it works.

Thanks!

Cool..I configured the environment variable on circleCI. And looks like CircleCI failed again. Not sure what is wrong in the config as it seems to be happening only with this PR.

working_directory: ~/user-data-for-fraud-prevention
docker:
- image: circleci/node:10.15.3-browsers

orbs:
Copy link
Contributor

@skodamarthi skodamarthi Oct 7, 2020

Choose a reason for hiding this comment

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

Could the location of orbs be a problem? Before your changes, It was placed at the beginning of the file. Not sure but just wondering if it is the cause for circleCI build failure..

@tylerkrupicka
Copy link
Contributor Author

Reading the docs it seems like it may throw the error because I changed the config version to get newer syntax, which might make you re-allow third party orbs.

Can you see if https://circleci.com/docs/2.0/orbs-faq/#using-uncertified-orbs helps?

@skodamarthi
Copy link
Contributor

Reading the docs it seems like it may throw the error because I changed the config version to get newer syntax, which might make you re-allow third party orbs.

Can you see if https://circleci.com/docs/2.0/orbs-faq/#using-uncertified-orbs helps?

@tylerkrupicka I am not an admin for Intuit Organisation, so I am not able to change this setting. Is there any other way we can do this without having to change the setting?

@tylerkrupicka
Copy link
Contributor Author

Well going back to your old config worked, although test coverage is throwing some sort of exception. Should I just leave the circle ci config unchanged, and then one of you can add the step for auto shipit. I'm not sure how circle handles changes to the build config in a PR.

@skodamarthi
Copy link
Contributor

Well going back to your old config worked, although test coverage is throwing some sort of exception. Should I just leave the circle ci config unchanged, and then one of you can add the step for auto shipit. I'm not sure how circle handles changes to the build config in a PR.

@tylerkrupicka Can the problem be because of adding 'auto shipit' to build job instead of release job in circle ci? Can you try moving it to release? I was just referring to circle ci configs in other intuit repos and have noticed that everywhere this step is done as part of release job. Eg: https://github.com/intuit/ReplayZero/blob/3faa5a68a44f3a8886ba359e19e7861035a2454f/.circleci/config.yml#L13

@tylerkrupicka
Copy link
Contributor Author

Maybe related to these: lemurheavy/coveralls-public#632
lemurheavy/coveralls-public#1264

Did you change the token for coveralls when you changed the GH token? The weird thing is that would cause it to fail for all jobs on your Circle.

@skodamarthi
Copy link
Contributor

skodamarthi commented Oct 12, 2020

@tylerkrupicka I tried now by regenerating the token for coveralls and adding the new token to circleCI environment variables. Still see the same build issue on re-running the build! Can you please get the latest code, update the version number in circleCI config and try committing again? I tried doing a commit to the repo sometime back after updating the tokens and the build worked fine for that commit. So, some issue with just this PR.

@reubenae
Copy link
Contributor

What are the next steps on this PR? Can we merge it?

@tylerkrupicka
Copy link
Contributor Author

tylerkrupicka commented Oct 19, 2020

@reubenae could probably use some help. Your coveralls integration is failing for this PR, but I didn't change anything related to it. It also appears Josh has the same problem on his PR. Since Josh is experiencing it too I'm kind of wondering if the builds fail when users make a PR from a fork.

I think this is good to go otherwise, I'm just not sure how to help on Coveralls. Let me know if you need me to do anything! I've tried changing the config yaml, using the yarn lock from master, etc but nothing has worked.

@reubenae
Copy link
Contributor

Looks like the fork thing is an issue: ethereumjs/ethereumjs-monorepo#359

I'll check with Susmitha tomorrow but I suggest we merge this PR, if the master build passes we're good, if it fails I'll revert the merge. We'll probably need to disable coveralls if this is something it is going to do.

@skodamarthi
Copy link
Contributor

@tylerkrupicka @reubenae I agree. I tried multiple approaches last week too by regenerating tokens, logging in/ logging out, removing and adding project again to coveralls etc; Nothing seemed to work for getting a successful build with these two PRs. We can merge these and I can disable coveralls from the ci build for now until we figure out the issue.

@skodamarthi skodamarthi merged commit fb7e818 into intuit:master Oct 19, 2020
@skodamarthi
Copy link
Contributor

@tylerkrupicka After merging the PR, the master build was successful but the release failed with the below error. Any idea what could be wrong?
Screen Shot 2020-10-21 at 23 28 47

@tylerkrupicka
Copy link
Contributor Author

@skodamarthi I think that was an oversight on my part, since auto is not being installed as a global name. change that line to yarn run auto shipit.

@reubenae
Copy link
Contributor

@tylerkrupicka done #84 (cc @skodamarthi merge if you're happy)

@skodamarthi
Copy link
Contributor

@tylerkrupicka We tried updating to "yarn run auto shipit" but it still failed with the below error:
Screen Shot 2020-10-22 at 10 55 09

@tylerkrupicka
Copy link
Contributor Author

@skodamarthi Hmm yeah this is weird, I looked and actually we shouldn't have even had to make the other change, since scripts in package json already yarn run. I'm wondering if it's because in your CircleCI config there isn't an install step. I'm not super familiar with Circle, but maybe since you install in the build step, it's not available in the install step. Can you try making install a separate step that runs install, and make it a requirement for build and release?

I had it step up like that at one point but removed it when we were debugging coveralls.

Here's an example: https://github.com/intuit/ts-readme/blob/master/.circleci/config.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants