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

Clean up datasource template #5220

Open
2 of 7 tasks
bajtos opened this issue Apr 24, 2020 · 9 comments
Open
2 of 7 tasks

Clean up datasource template #5220

bajtos opened this issue Apr 24, 2020 · 9 comments
Assignees
Labels
good first issue Hacktoberfest Tasks ready for new contributors to work on tech-debt

Comments

@bajtos
Copy link
Member

bajtos commented Apr 24, 2020

This is a follow-up issue for #5000 where we moved datasource config from JSON files to TS files and discovered few aspects of the datasource template that can be improved.

Acceptance criteria

For each of the items above, consider the following places to update


🎆 Hacktoberfest 2020

Greetings 👋 to all Hacktoberfest 2020 participants!

Here are few tips 👀 to make your start easier, see also #6456:

  • Before you start working on this issue, please leave a comment to let others know.
  • This issue consists of several tasks to work on, it may feel like a too big effort to undertake. Don't worry! It's perfectly fine to pick just one item from the list and leave the rest for somebody else. In fact, we prefer to have a dedicated pull request for each part, to make it easier for us to review the changes and get the pull request landed faster. Baby steps FTW! Remember, every little helps.
  • If you are new to GitHub pull requests, then you can learn about the process in Submitting a pull request to LoopBack 4.
  • If this is your first contribution to LoopBack, then please take a look at our Developer guide
  • Feel free to ask for help in #loopback-contributors channel, you can join our Slack workspace here.
@mrmodise
Copy link
Contributor

mrmodise commented Oct 5, 2020

@bajtos am interested in contributing on this. Maybe start with point 1 & 2 as a starter since am new to Lb contribution

@bajtos
Copy link
Member Author

bajtos commented Oct 6, 2020

am interested in contributing on this. Maybe start with point 1 & 2 as a starter since am new to Lb contribution

@mrmodise awesome 👍🏻 Starting with the first two items is fine, I'll review your pull request the other issue you created in a moment.

@mrmodise
Copy link
Contributor

mrmodise commented Oct 6, 2020

am interested in contributing on this. Maybe start with point 1 & 2 as a starter since am new to Lb contribution

@mrmodise awesome 👍🏻 Starting with the first two items is fine, I'll review your pull request the other issue you created in a moment.

Thanks raised PR but having issues with grPc connection and Travis CI commit-linting

@mrmodise
Copy link
Contributor

mrmodise commented Oct 6, 2020

Also wanted to get your thoughts on having the config object inside the datasource.ts. What I discovered on our projects at work is that these tend to grow in size for complex projects (ours are around 400 to 500 lines). Would it not be ideal to move these ito a config folder and have them listed underneath here following the same conventions as when we were using JSON files?

@bajtos
Copy link
Member Author

bajtos commented Oct 6, 2020

What I discovered on our projects at work is that these tend to grow in size for complex projects (ours are around 400 to 500 lines).

Interesting. What kind of configuration or code are you putting into your datasources? I would like to better understand your use case.

@mrmodise
Copy link
Contributor

mrmodise commented Oct 6, 2020

What I discovered on our projects at work is that these tend to grow in size for complex projects (ours are around 400 to 500 lines).

Interesting. What kind of configuration or code are you putting into your datasources? I would like to better understand your use case.

We don't change the default config on the datasource Classes. The only change we do is we import the config from a different file since these will be longer due to many systems we are integrating with.

so in our datasources we only change the dsConfig to point to the imported config, which will have many operations thus the longer length

@bajtos
Copy link
Member Author

bajtos commented Oct 9, 2020

which will have many operations thus the longer length

IIUC, you are using loopback-connector-rest? I see how a large configuration of operations can make the datasource file difficult to maintain.

Personally, I would keep options like connector and baseURL inside the main .datasource.ts file and move only operations into a different file (or multiple files).

Maybe we can introduce some sort of a convention for loading definitions of REST operations from external files, perhaps there can be one file per operation template? I think this would be best discussed in a new GitHub issue though.

@mrmodise
Copy link
Contributor

mrmodise commented Oct 9, 2020

operations

Thanks. Will log an issue and move the conversation over there

@mrmodise
Copy link
Contributor

mrmodise commented Jan 2, 2021

@bajtos when you're back I would like for us to close all the items on this issue. Thanks

@mrmodise mrmodise self-assigned this Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Hacktoberfest Tasks ready for new contributors to work on tech-debt
Projects
None yet
Development

No branches or pull requests

2 participants