-
Notifications
You must be signed in to change notification settings - Fork 8
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
fix: Tag fetching #27
fix: Tag fetching #27
Conversation
The previous code version would only check the first API tags page, meaning tag couldn't be found if not located on the first API page.
I'm maybe gonna split this PR up a bit before merging ^^ Also, I think for unit testing there's some frameworks like |
I feel like this is overkill tbh, the idea was to ensure new function behaves properly during development. |
forgot to change it in merge, whoops
So that IDEs can pick it up properly etc
I'm gonna remove the testing into separate PR and then merge this one ^^ |
Will be in separate 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.
Aight, I think I have massacred this PR enough now xD
Anyway, code works and as far as I can tell the logic is correct ^^
The unit testing code for this will be added in a separate PR.
Also I'll change the |
META PR, NO MOD VERIFICATION NEEDED
While checking tags, if current tag isn't found in first page, script will now browse all API pages until either tag is found or there are no more pages.
I also added a few tests to ensure correct method behaviour.Moved to #34Closes #17.