Conversation
|
Hi! I'd like some feedback on this, please.
|
|
@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! |
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 google#490.
cf0a311 to
fb41801
Compare
|
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 |
|
@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 |
|
Looking into the Windows test timeout... |
|
@richardbarrell-calvium Nice! Looks like it passed. Do you mind pulling from |
|
I've merged main and pushed again. |
Rationale:
node_modules/eslint/...may not exist, because the module may have been installed in anothernode_modulesfurther up the filesystem tree. Yarn does ensure that the binary gets symlinked intonode_modules/.bin.)