-
Notifications
You must be signed in to change notification settings - Fork 73
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
Typescriptify the ClientFactory #186
Conversation
6e5e714
to
1508a27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far, but you've used any
and the current linter config told me not to use it. Are we ok with some explicit any
s?
I would be, but of course I would avoid them where possible.
Also, the linter seems to not allow any warnings, which is odd. We should allow warnings, and turn the rules to error
if we actually want them to fail CI.
@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and | |||
limitations under the License. | |||
*/ | |||
|
|||
module.exports.ClientFactory = require("./components/client-factory"); | |||
export * from "./components/client-factory"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I need to do this for Intent in #185?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably yup, do the tests pass without it?
Co-authored-by: Christian Paul <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
No description provided.