gts

gts copied to clipboard

Rationale:

• npm and Yarn both put (a directory containing) the eslint binary on PATH during "npm run foo", so take advantage of that to invoke eslint in a way that is compatible with Yarn workspaces.
• (Node that node_modules/eslint/... may not exist, because the module may have been installed in another node_modules further up the filesystem tree. Yarn does ensure that the binary gets symlinked into node_modules/.bin.)
• This should fix issue #490.

Hi! I'd like some feedback on this, please.

• Does this work with everyone else's workflow? I believe it works fine with using gts by running the various scripts like npm run lint, npm run test and so on. I don't know for certain if there are other ways in which people invoke gts in the wild?
• Were there any performance issues or anything like that which were being dealt with previously by invoking eslint from node_modules/eslint/bin/eslint?
• I'm not aware of any reason why this in principle shouldn't work on Windows, but I don't have access to a Windows box to test this on. (I can get ahold of one in a few hours.)

@JustinBeckwith would you be a good person from whom to request code review on this, please? Tagging you because you mentioned PRs being welcome on the issue (#490) that I'm trying to solve.

@richardbarrell-calvium this change seems reasonable to me, but I would like to test in a few environments before landing.

Could I bother you to rebase and push, for whatever reason it seems like actions did not kick off.

Alternatively, @richardbarrell-calvium, if you could check the box that grants contributor write access to your branch, we can drive this to completion. Thank you!

Hi there! @bcoe apologies for not replying sooner, the email notification got buried in my inbox. My bad.

@alexander-fenster I've rebased my branch on main and pushed it. I'm afraid I have no idea where that checkbox is. I can grant you write access to https://github.com/calvium/gts from the "Collaborators and teams" page?

GitHub's UI is telling me that the CI actions weren't invoked because workflow approval is required. Here is a screenshot of what I'm seeing:

@alexander-fenster It would be great to see this move forward. If you're worried about compatibility, you could append ./node_modules/eslint/bin to PATH or manually fallback to the original invocation?

@danielbankhead could you please have a look at this one?

@richardbarrell-calvium please pull from the base branch and I can get this merged.

@danielbankhead Thanks! I've merged main in to my branch and pushed.

Looking into the Windows test timeout...

@richardbarrell-calvium Nice! Looks like it passed. Do you mind pulling from main again?

I've merged main and pushed again.