-
-
Notifications
You must be signed in to change notification settings - Fork 362
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
Homepage changes - Modify Connections to Databases #3710
Conversation
…ponse instead of just the ids
…tabaseConnectionResult
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.
This looks great! Nice work on this. I feel like databases.ts
is basically exactly as I would have written it. Cool!
I don't have any concerns with the work in this PR, but I do have one question regarding what follows this:
-
I notice a few things together which make me curious:
- The
Database
type hasserver_id
instead ofhost
andport
properties. - We have a separate
Server
type - When call
api.database_setup
RPC methods, we are currently discarding theserver
property returned from the API.
This makes me wonder how you're planning to persist that server data in the front end in order to display it. The impression I get is that you're planning to have a separate
ServersStore
. Is that right? This is the kind of architectural stuff that I think would be nice to sync up about somehow before you launch into coding. If you could lay out a rough outline of how you want these stores to work on the front end, I think it would be really helpful for us both to make sure we're on the same page about the plans. TheDatabasesStore
looks great as it is on its own. But I'd like the opportunity to give input on the larger stores structure before you get so far along that making large structural changes becomes difficult. - The
Related to #3687
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin