-
Notifications
You must be signed in to change notification settings - Fork 22
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
Make merging PRs and updating IDs easier with a node script that fixes duplicate IDs and orders them in the JSON data #11
Comments
Hi @EddyVinck |
Sure @taksuparth go ahead! You can message me here or on Slack if you have any questions. |
As discussed on Slack, I'm building a proof of concept which: Replaces the auto-increment IDs with random generated ones Adding a simple CLI tool which helps the contributor to add new content to the DB |
Check my initial implementation here: #12 |
@dios-david I just tried it out, and like I already said on Slack the code is organized really well! I don't really have much experience from coding CLIs but I definitely learned a thing or two just by looking at the code. Nice job 👍 The CLI itself is pretty good for the amount of time you put into it so far. This I could really see this turning out as a great foundation for a GUI version for the website like you suggested. I don't know if you've ever used something like Docusaurus (we use this for https://falcon.deity.io), but they have this nice button that forks the original repo and creates a new branch for you to work on. If we could automate that combined with a GUI editor that merges in the new data with the current data I think we would have a great solution. What I'm currently not seeing is a way to make working with merge conflicts easier. The challenge here is that the entire database is now 1 file ( Another challenge I see to solve this problem is that it's not really possible to differentiate the new data in the PR from the data in the master branch. Maybe analyzing the characters added in merge conflicts could solve this, but I'm not sure if that is a good approach. Another option could be to add a Some quality of life improvements could be:
Honestly, great work so far. I'm really interested to see where we can take this 🙂 |
This has some level of crossover with the issue I just raised. I'd be interested in your thoughts #43 |
@jglover I like the suggestion! Although I'm not sure if this is the right approach since we're already looking into a JSON "database" in the issue you referenced. We could add interfaces like you suggested, while still keeping the data in a JSON file managed by You raise valid concerns about the data, like maintaining data integrity, merge conflicts and refactoring workloads which I hope to tackle with tests, scripts and perhaps even a bot on GitHub after we get this JSON database approach working the way we want. The tests can be run with Travis CI, which is free for open source projects. The bot could update and auto-merge the latest master branch into PRs. When the CLI is working I hope we can take that to the next level in the form of a GUI on the website to provide a simple and accessible way of adding data. My main concern with swapping our JSON approach with a TypeScript approach, is it being accessible to many developers. A lot of people from different development backgrounds understand JSON, while only a fraction will understand TypeScript. |
@dios-david I hope to take a look at your PR again later today after fixing this bug: #45 If not today then probably tomorrow. As for the CLI, I'd like to explore adding an option that fixes a merge conflict. If I have time I might also look into options like searching for a speaker name of category. I'll probably make a PR to your branch. Writing a CLI is not something I'm very experienced with so this should be a fun learning experience 🙂 |
I just created a new label "critical" for issues with a high priority. I'd like to prioritize this feature over other features for now. Life if pretty busy in and outside of work right now, so thank you for your patience. I'll start working on this when I can. |
In the data directory you will find a few files with relational data. Since this repository depends on pull requests to add more data and the fact that multiple pull requests need to be merged at a time is a problem that needs to be solved.
A few pull requests in and I'm already feeling the problem with the merge conflicts. If there was a node script that would fix duplicate IDs, auto-increment them and order them in a logical way that would really help people contribute and save everyone a lot of time.
The text was updated successfully, but these errors were encountered: