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

move tooling into root & stop shipping eslint with CLI #46

Merged
merged 6 commits into from Aug 9, 2019

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Aug 8, 2019

rOk! So this is tacking some monorepo tooling debt we hadn't gotten around to doing. The PR is pretty noisy, so i'll summarize the important changes:

  1. added a root .eslintrc
  2. in each of the 4 main package.jsons:
    a. moved lint, test, and smoke-test into a single command: validate
    b. removed posttest: "yarn lint"
  3. added zapier-platform-example-app- to the name of the 3 oauth1 examples. This doesn't change init behavior (we were looking them up by folder name, not package.json/name), but does let us easily ignore them in the root lerna commands
  4. removed .eslintrc and yarn.lock from all sub-packages
  5. pulled a lot of devDeps out of sub packages so they'll all use the same version in root
  6. removed eslint app linting from CLI (only actual code change)

Notes

  1. I've verified locally that tests pass.
  2. Linting (and thus CI) will fail, since I didn't want to add more noise to already noisy PR. Once it's ✅, i'll fix all the linting issues. it's mostly using const instead of let and spacing after comments (// asdf instead of //asdf).
  3. We should probably mark master branch as protected, since this is not the first time I've accidentally pushed straight there. I think i've scrubbed that history correctly.

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for cleaning this up!

On your note (3), yes, I think we can mark master branch as protected. Feel free to go for it.

@@ -1053,12 +1053,12 @@ How will we get a list of new objects? Will be turned into a trigger automatical

* ```
{ display: { label: 'New User', description: 'Trigger when a new User is created in your account.' },
operation:
operation:
Copy link
Member

Choose a reason for hiding this comment

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

The trailing spaces were re-added in my last PR. I don't know why they keep coming back. 🤷‍♂ Maybe your IDE automatically strips trailing spaces for all files in the repo? Mine only does that to a file when it's saved.

@xavdid xavdid changed the title move tooling into root move tooling into root & stop shipping eslint with CLI Aug 9, 2019
@xavdid xavdid merged commit 21d6761 into master Aug 9, 2019
@xavdid xavdid deleted the tooling-for-real branch August 9, 2019 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants