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 data team report #377

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Fix data team report #377

merged 1 commit into from
Nov 3, 2023

Conversation

inglesp
Copy link
Contributor

@inglesp inglesp commented Nov 3, 2023

We'd made too many assumptions about the ordering of result from GH's API.

No new tests sinc existing code is untested.

@inglesp
Copy link
Contributor Author

inglesp commented Nov 3, 2023

At least, I assumed it didn't have tests because the changed function has pragma: no cover, but the build's failing because coverage isn't 100%.

Copy link
Contributor

@evansd evansd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a sensible change to me.

I think the coverage failure is only happening because there's an actual test failure first. And that's probably because the test data doesn't include the field.name attribute.

We'd made too many assumptions about the ordering of result from GH's
API.
@inglesp inglesp merged commit d8d3fa5 into main Nov 3, 2023
3 checks passed
@inglesp inglesp deleted the fix-data-team-report branch November 3, 2023 12:06
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.

None yet

2 participants