atom-linter icon indicating copy to clipboard operation
atom-linter copied to clipboard

feat: convert to TypeScript

Open aminya opened this issue 3 years ago • 4 comments

Closes #252 #253 #254

Includes the types from this PR. We can remove them here once that PR is merged. https://github.com/steelbrain/exec/pull/103

aminya avatar Mar 19 '21 11:03 aminya

@steelbrain Please take a look at this.

aminya avatar Mar 19 '21 14:03 aminya

Thank you so much for working on this!

You're welcome!

Appreciate the work here. I have some reservations about some of the tooling choices, can we please use npm or yarn instead of pnpm? Personally love pnpm but for public projects, it adds a barrier to entry IMO

That's exactly why I removed the package-lock.json file and added the npmrc file. This gives people the freedom to use any package manager they like such as npm, yarn, or pnpm. I actually didn't intend to commit the pnpm-lock.yaml, but using the current config, pnpm handles out-of-date lock-file without issue, so people can update dependencies without breaking anything. I can remove this lock file if you prefer that.

Also some of the changes may be behavior changes, although they look harmless from a first glance, but it's possible they'll behave differently. Can we split the rewrite to TS and those changes into separate PRs or do you think they should go together?

Yes, I always keep my commits specific, completely composable, and incremental to allow future rebasing, reverting, and cherry-picking. So, if you can give me the commit range that you would like me to move, I will happily move it to a follow-up PR. What I did in this PR are:

  • Convert to TypeScript
  • Fixing the tsc error
  • Fixing eslint errors

I kept everything backward compatible, but still, feel free to give me the range that you would like me to move.

aminya avatar Mar 20 '21 03:03 aminya

Any feedback on the answers? @steelbrain

aminya avatar Mar 23 '21 21:03 aminya

@aminya Generally looks good to me, I'll run/test this and then post further feedback (if any) then we can merge

steelbrain avatar Mar 23 '21 21:03 steelbrain