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

fix: Tag fetching #27

Merged
merged 16 commits into from
Jan 30, 2024
Merged

Conversation

Alystrasz
Copy link
Contributor

@Alystrasz Alystrasz commented Jan 25, 2024

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 #34

Closes #17.

@GeckoEidechse GeckoEidechse self-requested a review January 25, 2024 21:58
@GeckoEidechse
Copy link
Member

I'm maybe gonna split this PR up a bit before merging ^^

Also, I think for unit testing there's some frameworks like pytest that make it a bit nicer so that we don't have to pass a CLI arg to run unittest. Then we can also combine it with our current CI to always run meta tests for verification scripts ^^

@Alystrasz
Copy link
Contributor Author

Then we can also combine it with our current CI to always run meta tests for verification scripts

I feel like this is overkill tbh, the idea was to ensure new function behaves properly during development.

@GeckoEidechse
Copy link
Member

I'm gonna remove the testing into separate PR and then merge this one ^^

Copy link
Member

@GeckoEidechse GeckoEidechse left a 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.

@GeckoEidechse
Copy link
Member

Also I'll change the .format() parts to f-strings as well in a separate PR ^^

@GeckoEidechse GeckoEidechse merged commit d0f95df into R2Northstar:master Jan 30, 2024
3 checks passed
@Alystrasz Alystrasz deleted the fix/tag-fetching branch July 10, 2024 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tag fetching
2 participants