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

docs: add AndEsterson as a contributor for infra #4674

Closed
wants to merge 2 commits into from

Conversation

allcontributors[bot]
Copy link
Contributor

Adds @AndEsterson as a contributor for infra.

This was requested by Saransh-cpp in this comment

[skip ci]

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

Still requires "style" but it can sorted later

@agriyakhetarpal
Copy link
Member

I feel that if we want more checks to be required, maybe the better approach is to require jobs other than the style job instead (i.e., the unit and integration tests). The last time I tinkered with this setting, it was possible to set regex patterns for rulesets, so we can configure this for platforms and Python versions, and never need to bother with them again.

Though, one downside I can think of is that an administrator will have to come in to merge PRs with any failing checks that are irrelevant to the changes made in them, such as flaky tests, dependency download failures, etc.

@kratman
Copy link
Contributor

kratman commented Dec 13, 2024

Though, one downside I can think of is that an administrator will have to come in to merge PRs with any failing checks that are irrelevant to the changes made in them, such as flaky tests, dependency download failures, etc.

We should just require checks for as many CI tasks as possible, it is rare that a test should fail from a PR. Flaky tests should be fixed, problematic dependencies should be replaced, bots that cause breakages should be removed.

I find it very weird that we don't require tests to pass before merging. We have some complicated infrastructure and don't actually enforce it.

On the other side, I put up another fix for this, so I am going to close this PR so I can test the fix

@kratman kratman closed this Dec 13, 2024
@kratman kratman deleted the all-contributors/add-AndEsterson branch December 13, 2024 18:45
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.

3 participants