Closed brandongregoryscott closed 3 years ago
Merging #130 (7680e9a) into release/2.0.0 (dbfd744) will decrease coverage by
2.18%
. The diff coverage is79.59%
.
@@ Coverage Diff @@
## release/2.0.0 #130 +/- ##
=================================================
- Coverage 77.50% 75.32% -2.19%
=================================================
Files 28 27 -1
Lines 1218 1244 +26
Branches 182 243 +61
=================================================
- Hits 944 937 -7
- Misses 239 297 +58
+ Partials 35 10 -25
Impacted Files | Coverage Δ | |
---|---|---|
src/tests/test-utils.ts | 30.48% <37.28%> (ø) |
|
src/modules/echo.ts | 21.27% <37.50%> (ø) |
|
src/modules/prompt.ts | 35.29% <46.15%> (ø) |
|
src/modules/dotnet-test.ts | 44.91% <56.66%> (ø) |
|
src/modules/github.ts | 68.89% <67.81%> (ø) |
|
src/modules/formatters.ts | 73.33% <73.33%> (ø) |
|
src/modules/git.ts | 62.06% <78.57%> (ø) |
|
src/modules/nuget-upgrade.ts | 74.13% <80.00%> (ø) |
|
src/modules/file.ts | 60.00% <85.71%> (ø) |
|
src/utilities/command-string-builder.ts | 85.71% <85.71%> (ø) |
|
... and 32 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 dbfd744...0a55fbb. Read the comment docs.
@brandongregoryscott going to try this out this afternoon, but after reading it through it looks awesome! phew, nice work 👏
@wintondeshong I believe (at least part of) the slowdown is from the exports in and-cli.ts
. I added those in pretty late in the port which is probably why I didn't notice it. If you comment out all of those, rebuild and run and-cli-dev github
, the time goes from ~5s to ~1s:
bscot@BSCOTT-PC ~/acc/fork/brandongregoryscott.AndcultureCode.Cli/ (feature/typescript) -> time and-cli-dev github
...
real 0m4.320s
user 0m0.015s
sys 0m0.015s
bscot@BSCOTT-PC ~/acc/fork/brandongregoryscott.AndcultureCode.Cli/ (feature/typescript) -> time and-cli-dev github
...
real 0m1.130s
user 0m0.015s
sys 0m0.015s
...which sucks. I was hoping to improve the consumer experience by exporting all of the available modules, but maybe there's another approach we can take.
Update: moved all of the exports to index.ts
which is the new "main" file for the package. This should allow us to have easy imports + comparable speed to the pre-TS commands
@wintondeshong I believe (at least part of) the slowdown is from the exports in
and-cli.ts
. I added those in pretty late in the port which is probably why I didn't notice it. If you comment out all of those, rebuild and runand-cli-dev github
, the time goes from ~5s to ~1s:bscot@BSCOTT-PC ~/acc/fork/brandongregoryscott.AndcultureCode.Cli/ (feature/typescript) -> time and-cli-dev github ... real 0m4.320s user 0m0.015s sys 0m0.015s
bscot@BSCOTT-PC ~/acc/fork/brandongregoryscott.AndcultureCode.Cli/ (feature/typescript) -> time and-cli-dev github ... real 0m1.130s user 0m0.015s sys 0m0.015s
...which sucks. I was hoping to improve the consumer experience by exporting all of the available modules, but maybe there's another approach we can take.
Update: moved all of the exports to
index.ts
which is the new "main" file for the package. This should allow us to have easy imports + comparable speed to the pre-TS commands
I'm going to step through this here because mine isn't a matter of seconds. It finishes running the actual work from the command (ie. workspace -u wintondeshong
) and then hangs for upwards of 30 seconds to a minute.
I'm going to step through this here because mine isn't a matter of seconds. It finishes running the actual work from the command (ie.
workspace -u wintondeshong
) and then hangs for upwards of 30 seconds to a minute.
Let me know what you can find. If you have some free time this week, we can try to pair up on it. I have not seen that level of slowness on my Windows machine or Macbook, before or after the export changes.
Here is my current timing...
07:42 /c/Clients/andculturecode $ time and-cli-brandongregoryscott workspace -u wintondeshong
[and-cli] -----------------------------------------------------------------
[and-cli]
[and-cli] Configuring workspace
[and-cli]
[and-cli] -----------------------------------------------------------------
[and-cli] Synchronizing forks of AndcultureCode for 'wintondeshong'...
[and-cli] Results
[and-cli] - Successful: 0
[and-cli] - Unmodified: 24
[and-cli] - Errored: 0
[and-cli] - Total: 24
real 1m14.457s
user 0m0.015s
sys 0m0.015s
@brandongregoryscott something seemed up so I re-cloned and did a fresh build of your branch and I'm no longer seeing that performance issue 🤷♂️
05:35 /c/Clients/andculturecode $ time and-cli-brandongregoryscott workspace -u wintondeshong
[and-cli] -----------------------------------------------------------------
[and-cli]
[and-cli] Configuring workspace
[and-cli]
[and-cli] -----------------------------------------------------------------
[and-cli] Synchronizing forks of AndcultureCode for 'wintondeshong'...
[and-cli] Results
[and-cli] - Successful: 0
[and-cli] - Unmodified: 24
[and-cli] - Errored: 0
[and-cli] - Total: 24
real 0m2.080s
user 0m0.031s
sys 0m0.000s
@brandongregoryscott something seemed up so I re-cloned and did a fresh build of your branch and I'm no longer seeing that performance issue 🤷♂️
So weird 🤔 well, glad it's not an issue!
@brandongregoryscott with the performance issue no longer an issue I'm good to approve. I tested the install and nuget --publish commands as well, which leaves basically the deploy series of commands. Recommend getting someone to test those out who might have them in action presently, which would save us time testing.
Awesome! Thanks for helping out with the testing process. I'll reach out to some of the engineers that are leveraging the remaining deployment commands to see if they have capacity to smoke test the changes.
Closes #14 Investigate feasibility/benefit of porting project to Typescript
Initial pass of the TS port for the project. All automated tests should be updated & passing. There are likely some missed areas of type narrowing/specification that can be filled in later.
A few things of note:
ts-node
for development. I was unable to use justts-node
to run anything but the main entrypoint file, and subcommands were not registered. It required a much longer command string pointing to the tsconfig file (see https://github.com/tj/commander.js/issues/955#issuecomment-490016192 for an example)watch
npm script as theand-cli-dev
alias now points to<repo>/dist/and-cli.js
.ts
, but theTestUtils
function that executes the CLI commands will also reference thedist
folder for executable js files.ts-jest
is throwing warnings about using jest <26. I believe jest v26+ requires node 10 and above, so we'd need to agree to move forward with that upgrade as a team - there are some implications with our Jenkins server that is still on node 8.16.3CoreUtils
,CollectionUtils
, etc.). So any plugin projects or older branches referencing them will need to be updated.and-cli.ts
) will now export all of the shared modules for consumer use, similar to how ourandculturecode-javascript-core
andandculturecode-javascript-react
packages export everything in theindex.ts
file.@wintondeshong This is a massive PR, so I don't expect you to scan everything line-by-line (some of the restructuring caused git to register the changes as new files entirely), but I'd love to get your input on some of this, too. In terms of releasing it, I was thinking we could release it as
2.0.0-beta
version(s) until we iron out any of the potential regressions with the port - what do you think?ts-jest
warningsInstructions for beta-testing
``` # Ensure latest and-cli is installed for access to workspace command npm i -g and-cli@latest # Ensure you are in a directory appropriate for holding OSS repos (can be anywhere, just an example) cd ~/andculturecode/forks/ # Clone all of my forks and-cli workspace -u brandongregoryscott && cd brandongregoryscott.AndcultureCode.Cli # Fetch remote branches and checkout the typescript branch git fetch && git checkout feature/typescript # Install dependencies and run a build npm install && npm run build # Install the CLI aliases ./dist/and-cli.js install Now you can run the built JS version of the CLI by running `and-cli-dev` ```Command audit/testing checklist
- [x] `copy` - [x] `-d, --destination