forge icon indicating copy to clipboard operation
forge copied to clipboard

fix: vscode debugging shell file

Open wesyao opened this issue 2 years ago • 9 comments

  • [x] I have read the contribution documentation for this project.
  • [x] I agree to follow the code of conduct that this project follows, as appropriate.
  • [x] The changes are appropriately documented (if applicable).
  • [x] The changes have sufficient test coverage (if applicable).
  • [x] The testsuite passes successfully on my local machine (if applicable).

Summarize your changes: The debugging experience is broken on mac/linux and was introduced #535.

There has been a request for this issue #1369.

wesyao avatar Jul 12 '22 20:07 wesyao

Codecov Report

Merging #2908 (9d7d4a0) into master (345250c) will decrease coverage by 0.12%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2908      +/-   ##
==========================================
- Coverage   71.82%   71.69%   -0.13%     
==========================================
  Files          79       79              
  Lines        2385     2392       +7     
  Branches      449      450       +1     
==========================================
+ Hits         1713     1715       +2     
- Misses        450      548      +98     
+ Partials      222      129      -93     
Impacted Files Coverage Δ
packages/utils/async-ora/src/ora-handler.ts 81.81% <0.00%> (-4.85%) :arrow_down:
packages/api/core/src/api/start.ts 64.04% <0.00%> (-1.84%) :arrow_down:
packages/maker/base/src/Maker.ts 69.69% <0.00%> (ø)
packages/api/core/src/api/make.ts 79.00% <0.00%> (ø)
packages/api/core/src/util/hook.ts 75.00% <0.00%> (ø)
packages/api/core/src/api/import.ts 61.76% <0.00%> (ø)
packages/api/core/src/api/package.ts 75.55% <0.00%> (ø)
packages/api/core/src/api/publish.ts 68.42% <0.00%> (ø)
packages/api/core/src/util/out-dir.ts 66.66% <0.00%> (ø)
packages/api/core/src/util/parse-archs.ts 60.00% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 345250c...9d7d4a0. Read the comment docs.

codecov[bot] avatar Jul 12 '22 20:07 codecov[bot]

It would be good if someone could test the following scenarios before merging:

  • standalone git repo on Linux
  • standalone git repo on macOS
  • monorepo on Linux
  • monorepo on macOS

malept avatar Jul 13 '22 16:07 malept

Additionally the PR title does not conform to conventional commits standards as per the contributing docs

malept avatar Jul 13 '22 16:07 malept

Thanks for taking a look @malept & @VerteDinde.

Can you help me define a test plan @VerteDinde? I want to make sure we're testing the exact same way to ensure parity. Also, if you could help me with Ubuntu, I'd greatly appreciate it (no linux environment for me). 😅

@malept , what differences are you seeing between a mono repo and a stand alone git repo?

wesyao avatar Jul 13 '22 20:07 wesyao

The node_modules folder exists in different places depending on the monorepo manager.

malept avatar Jul 13 '22 21:07 malept

So, you'd just like me to simulate a repo with various directories and the node_modules directory in some subfolder?

Something like this?

- Repo
|__folder 1
|__|__folder a
|__|__|__app
|__|__|__node_modules

I wanted to also point out that I'm really just reverting it back before this PR: #536

wesyao avatar Jul 13 '22 21:07 wesyao

I don't think simulation is very realistic, by definition. Using actual monorepo managers is preferred.

malept avatar Jul 13 '22 21:07 malept

Hey @VerteDinde, I followed up with @malept to see what specifically needs to be tested to verify this change. I don't have an Ubuntu environment. If you could help me test the Ubuntu flow, I'd appreciate it! 😄

Here is the test matrix that needs to be tested: pnpm, npm workspaces, yarn workspaces, lerna

Just to be transparent, this experience is currently broken and it is a documented as being supported here.

I'll tackle the macos experience and circle back.

Thanks to all!

wesyao avatar Jul 14 '22 01:07 wesyao

@wesyao Sure thing! I'll give these a test on Ubuntu next week (PST), and will report back here 🙂

VerteDinde avatar Jul 15 '22 19:07 VerteDinde

This should be resolved by https://github.com/electron-forge/electron-forge-docs/pull/131, so closing this out - if there's still a problem here, feel free to ping me and I'll re-open.

dsanders11 avatar Feb 07 '24 18:02 dsanders11