Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(auth server): typescript lib/oauth/metrics #5161

Merged
merged 1 commit into from May 7, 2020

Conversation

jodyheavener
Copy link
Contributor

Closes #4879

Part of a larger effort to adopt TypeScript in the Auth Server.


I'm trying to take a slow, educational approach to TypeScript, so please feel free to tear this apart.

@jodyheavener jodyheavener force-pushed the jh/4879/auth-ts/lib-oauth-metrics branch from b780fa5 to bc0db23 Compare April 30, 2020 03:19
@jodyheavener jodyheavener requested a review from a team April 30, 2020 03:37
@jodyheavener jodyheavener marked this pull request as ready for review April 30, 2020 03:38
@bbangert bbangert self-assigned this Apr 30, 2020
@@ -0,0 +1,11 @@
// This interface should be expanded on
// as we slowly move over to TypeScript
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually might be easier just change the config file to .ts, install @types/convict, and export typeof conf in the config ts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So many implications for changing the config file to typescript 😆

@jodyheavener jodyheavener force-pushed the jh/4879/auth-ts/lib-oauth-metrics branch 4 times, most recently from 343b0ef to f82a7bd Compare May 6, 2020 15:36
@chenba
Copy link
Contributor

chenba commented May 6, 2020

If you add "[skip ci]" to your commit message of a wip commit, CircleCI will skip CI.

@jodyheavener
Copy link
Contributor Author

@chenba fair, yeah, this config change has just been so far reaching that I'm using CI to find all the spots 😅

@jodyheavener
Copy link
Contributor Author

@chenba Okay, so we're passing now, but you'll notice I had to up certain timeouts an quite a bit.

I'm guessing it has to do with mocha using TypeScript and needing to compile everything repeatedly. Or perhaps it's the use npx (because it will install ts-node if it is not present), but because we npm i before the test I wouldn't suspect it's that. At any rate, I don't feel great about the timeouts. If you have any thoughts I'm all ears.

@jodyheavener jodyheavener force-pushed the jh/4879/auth-ts/lib-oauth-metrics branch from 86d7691 to 0e37797 Compare May 7, 2020 16:11
@jodyheavener jodyheavener force-pushed the jh/4879/auth-ts/lib-oauth-metrics branch from 0e37797 to 97cf82e Compare May 7, 2020 16:50
@jodyheavener jodyheavener merged commit 97cf82e into master May 7, 2020
@jodyheavener jodyheavener deleted the jh/4879/auth-ts/lib-oauth-metrics branch May 19, 2020 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[auth-server] Add Typescript to files in /lib/oauth/metrics
4 participants