Skip to content

Conversation

addaleax
Copy link
Collaborator

@addaleax addaleax commented Mar 4, 2021

Paired on this with Michael because it seemed like a good ticket to
pair on and we’re going to be doing the loading-files epic soon.

Paired on this with Michael because it seemed like a good ticket to
pair on and we’re going to be doing the loading-files epic soon.
Copy link
Collaborator

@mcasimir mcasimir left a comment

Choose a reason for hiding this comment

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

This is so amazing that I don't even know what to say :) ❤️

};

export type MongoshConfigProvider = {
export type MongoshIOProvider = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

just a non-important random observation: this type is getting to be a bit of a jack of all trades, did we consider having multiple interfaces that ties together only the methods that are naturally coupled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, so... we could definitely split it up, but I feel like this might be one of those things where it's okay to wait until we have a reason to do so (e.g. if we ever make MongoshRepl a public API, which seems possible)

No strong feelings though :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, me neither, keeping it like that is super fine!

@addaleax addaleax merged commit 11d4e52 into master Mar 4, 2021
@addaleax addaleax deleted the 386-dev branch March 4, 2021 18:43
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.

3 participants