Skip to content

Conversation

@guptapratykshh
Copy link
Contributor

Description
added data collection for six null columns in repo_info table -contributing_file, security_issue_file, changelog_file, keywords, pull_requests_enabled, and pages_enabled.

This PR fixes #2675

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

Copy link
Member

@sgoggins sgoggins left a comment

Choose a reason for hiding this comment

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

I would like to see Unit tests written for these functions in a way that validates the existence of the end point and incorporates the use of edge cases. i.e. valid cases with an empty URL, invalid cases where there is not a 404 but its empty, etc.).

'issues_enabled': data['hasIssuesEnabled'] if 'hasIssuesEnabled' in data else None,
'open_issues': data['issues']['totalCount'] if data['issues'] else None,
'pull_requests_enabled': None,
'pull_requests_enabled': 'true' if repo_data.get('has_projects') else 'false',
Copy link
Member

Choose a reason for hiding this comment

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

We are not using the .get syntax elsewhere. I am not sure I love the nesting, and I agree it is useful to get these data elements we previously ignored.

Can you share a rationale for this design choice?

Copy link
Contributor

Choose a reason for hiding this comment

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

get syntax is better because it wont crash if the key doesnt exist (will return a default value, which is none by default)

I use it all the time

@sgoggins sgoggins self-assigned this Jan 20, 2026
@sgoggins sgoggins added release Related to releasing a new version of Augur add-feature Adds new features labels Jan 20, 2026
@MoralCode MoralCode removed release Related to releasing a new version of Augur add-feature Adds new features labels Jan 21, 2026
@sgoggins sgoggins added the add-feature Adds new features label Feb 2, 2026
@MoralCode
Copy link
Contributor

I would like to see Unit tests written for these functions in a way that validates the existence of the end point and incorporates the use of edge cases. i.e. valid cases with an empty URL, invalid cases where there is not a 404 but its empty, etc.).

Unit tests for API dependent stuff is... a little trickier to do. especially if the function being tested is making the web calls on its own, not expecting the data to be passed into them.

@MoralCode
Copy link
Contributor

MoralCode commented Feb 4, 2026

This will also conflict with #3597. Im going to say that this should wait for that PR to merge, and then be adjusted to reuse the same function for fetching the correct domain name to use in Github API calls.

@MoralCode MoralCode added the waiting This change is waiting for some other changes to land first label Feb 4, 2026
@MoralCode
Copy link
Contributor

Hello! Just wanted to check in to see if you were still interested in helping the maintainers merge this PR. We noticed it has been a little while since this last had activity, and are considering closing it or taking it over if it remains in its current state.

Please react to or reply to this to confirm your interest in the next 7 days or let us know if you are no longer interested in this so we can best prioritize everyone's contributions.

Thanks!

@MoralCode MoralCode added the stale Stuff that's abandoned or not making forward progress and may need taking over/closing label Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add-feature Adds new features stale Stuff that's abandoned or not making forward progress and may need taking over/closing waiting This change is waiting for some other changes to land first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

repo_info table not populating certain columns

3 participants