-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Store a list of current repos which use the JSON Schema topic and when they were created #4 #10
Conversation
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.
Welcome, and thanks for your initial work on this.
I have left a few comments and questions.
Looking at the requirements on #4.
- Covered.
- I have updated this to specify using the timestamp format.
- Not covered. "Fully run" means all results without limit. There are about 1.8k results. This may take some time to run.
- Not covered. This is important because it can take some time to run, so the script should capture errors and continue running rather than crash and exit.
When you have updated and tested, please indicate this needs to be re-reviewed. Thanks.
Feel free to reach me on our Slack under the same username if you have questions.
deleted unnecessary file
This is my second qualification task work! |
This looks much improved. I'm scheduling some time to test it locally before it is merged. Thanks for your work on this. |
deleted unnecessary file
I rebased and force pushed to this PR. |
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.
Left a few suggestions, otherwise this is looking better.
I know GSoC candidates have been selected now, but it would be nice to know if you'd like to finish this PR or not. If not, that's totally fine, and I'm sure I can find someone else to finish it or do it myself. Let me know.
[after fixed] Hi Ben, Thank you for your insightful feedback! I appreciate the guidance and the opportunity to learn from real-world problems provided by this project. I've completed a git rebase and resolved the divergent commits as you've noticed. I've also re-requested a review of the PR to ensure everything aligns with the project's current needs. Regarding the API key update, your clarification was very helpful. I understand now the importance of designing tests that don't make real API calls and will take this as a key learning point. Thank you for your support and for providing a nurturing environment for newcomers like me. I look forward to continuing my contribution to the project and will keep in touch via Slack as needed. Best regards, |
Hey David, thanks for waiting on this. I spent a good part of the day yesterday trying to modify one of the tests to make it work correctly. I've changed approaches and should be updating this PR today. |
Thank you, Ben. I deeply appreciated your support:) |
…as opposed to checking a function waw called
Thanks for your work on this! Much appreciated. I made a few improvements to the testing. |
Resolves #4
This PR is for 'Store a list of current repos which use the JSON Schema topic and when they were created' #4
Requirements
Here's the work that needs to be done on this:
json-schema
github topic and no longer record the data related to the way back machineinto a file, without causing the script to crash, and allowing it to continue if appropriate.Stretch objectives:
and mock the file writing, and check it has the correct content for the API data mocked.(It was later decided that we should not mock the filesystem.)
- Source: #4 (comment)
What I have done
This commit includes several enhancements to the initial data script: