Skip to content

CLI: Convert helpers and src/lib to ts#557

Closed
MarkPollmann wants to merge 7 commits intoredwoodjs:mainfrom
MarkPollmann:convert_helpers_to_ts
Closed

CLI: Convert helpers and src/lib to ts#557
MarkPollmann wants to merge 7 commits intoredwoodjs:mainfrom
MarkPollmann:convert_helpers_to_ts

Conversation

@MarkPollmann
Copy link
Copy Markdown
Contributor

@MarkPollmann MarkPollmann commented May 18, 2020 •
edited by thedavidprice
Loading

related to Tracking Issue #523

As these are some of the first ts files and I don't know the project well yet it was quite difficult to pin down the exact types. I tried to guess them from the test file and other usages but feel free to correct them :)

It definitely needs some refinement once more files are converted...

MarkPollmann
MarkPollmann commented May 18, 2020
View reviewed changes
Comment thread packages/cli/src/commands/generate/helpers.ts
MarkPollmann
MarkPollmann commented May 18, 2020
View reviewed changes
Comment thread packages/cli/src/commands/generate/helpers.ts
@thedavidprice thedavidprice mentioned this pull request May 18, 2020
Closed
42 tasks
@MarkPollmann MarkPollmann changed the title CLI: Convert helpers to ts CLI: Convert helpers and src/lib to ts May 19, 2020
@MarkPollmann MarkPollmann force-pushed the convert_helpers_to_ts branch from 1cabe7c to bd88662 Compare May 19, 2020 13:00
@MarkPollmann MarkPollmann force-pushed the convert_helpers_to_ts branch from bd88662 to 0b08663 Compare May 19, 2020 13:10
@MarkPollmann
Copy link
Copy Markdown
Contributor Author

MarkPollmann commented May 19, 2020 •
edited
Loading

I typed the cli/src/lib folder as well. There are a few issues left which I didn't know how to type properly. I don't think they are showstoppers but maybe someone else could take a look :)

Just a heads-up, I needed to re-order functions in src/lib/index.ts as TS complained that functions were used before they were defined.

No idea why eslint complains about import ordering for files I didn't touch, though. 🤔

Edit: ran yarn lint --fix

@peterp
Copy link
Copy Markdown
Member

peterp commented May 19, 2020

I don't think we have an equivalent ordering rule in the JS world. I find it somewhat annoying, but it kinda makes sense.

peterp
peterp reviewed May 19, 2020
View reviewed changes
Comment thread packages/cli/src/commands/generate/helpers.ts Outdated
peterp
peterp reviewed May 19, 2020
View reviewed changes
Comment thread packages/cli/src/commands/generate/helpers.ts Outdated
peterp
peterp reviewed May 19, 2020
View reviewed changes
Comment thread packages/cli/src/commands/generate/helpers.ts Outdated
@peterp
Copy link
Copy Markdown
Member

peterp commented May 22, 2020

@MarkPollmann This is an amazing amount of work, thank you so much for this.

@kimadeline kimadeline mentioned this pull request May 31, 2020
Merged
@peterp peterp changed the base branch from master to main June 19, 2020 15:34
@peterp
Copy link
Copy Markdown
Member

peterp commented Jun 20, 2020

Hey @MarkPollmann, I've messed up on this one - I should've merged it ages ago! I'm going to branch off your branch and see what I can in.

@peterp peterp self-assigned this Jun 20, 2020
@peterp peterp removed their assignment Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peterp peterp peterp left review comments

+1 more reviewer

@thedavidprice thedavidprice thedavidprice left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

topic/cli

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MarkPollmann @peterp @thedavidprice