Skip to content

Conversation

@ReubenJ
Copy link
Contributor

@ReubenJ ReubenJ commented Jul 4, 2024

Pull Request

Proposed Changes

Skip date validation when the respective environment variable is set. Currently, omitting the dates causes the action to fail, so it's impossible to output a report that does not include whether a user is a new contributor.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing
  • If publishing new data to the public (scorecards, security scan results, code quality results, live dashboards, etc.), please request review from @jeffrey-luszcz

Reviewer

  • Label as either bug, documentation, enhancement, infrastructure, maintenance or breaking

Copy link
Member

@zkoppert zkoppert left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for adding!

I believe we also need to modify contributors.py where it has:

if start_date and end_date:

To have an else statement otherwise contributors will never be properly filled.

@zkoppert
Copy link
Member

zkoppert commented Jul 4, 2024

This is great! Thanks for adding!

I believe we also need to modify contributors.py where it has:

if start_date and end_date:

To have an else statement otherwise contributors will never be properly filled.

Oops! Ok this just isn't true. Contributors is calculated before the conditional so never mind.

@zkoppert zkoppert merged commit f64f2cb into github:main Jul 4, 2024
@ReubenJ
Copy link
Contributor Author

ReubenJ commented Jul 5, 2024

Yeah—one more test case for contributors.py probably wouldn't have hurt, but it does work with the fix! Thanks for the quick response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants