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

gains author Bolivar #24

Merged
merged 1 commit into from
Dec 15, 2024
Merged

gains author Bolivar #24

merged 1 commit into from
Dec 15, 2024

Conversation

jibarozzo
Copy link
Contributor

No description provided.

@jibarozzo jibarozzo requested a review from jdhoffa December 14, 2024 18:03
@jibarozzo jibarozzo linked an issue Dec 14, 2024 that may be closed by this pull request
3 tasks
@jibarozzo jibarozzo requested a review from asbates December 14, 2024 18:04
Copy link

@DrEntropy DrEntropy left a comment

Choose a reason for hiding this comment

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

LGTM. I need to get on this myself. as soon as I contribute something that is ;)

@jibarozzo
Copy link
Contributor Author

Not sure why checks fail.

@jibarozzo jibarozzo merged commit 68617f0 into main Dec 15, 2024
0 of 12 checks passed
@jibarozzo jibarozzo deleted the 14-description-gains-authors branch December 15, 2024 03:21
@jdhoffa jdhoffa mentioned this pull request Dec 16, 2024
@jdhoffa
Copy link
Member

jdhoffa commented Dec 16, 2024

Not sure why checks fail.

FYI, checks failed because the DESCRIPTION file was improperly formatted (handled in #27).
In general, we should not get in the habit of approving/ merging PRs to main when checks fail 😊

Another note, the DESCRIPTION file is super finicky. I always use usethis functions (e.g. usethis::use_author) to add info to it, as it automatically handles formatting.

@jibarozzo
Copy link
Contributor Author

Agreed. I didn't use that function. 😐

@DrEntropy
Copy link

Sorry about the premature approval.

@DrEntropy
Copy link

Also surprised merging wasn't blocked ?

@jibarozzo
Copy link
Contributor Author

It's my bad for not thinking more and juast merging. :/

@jdhoffa
Copy link
Member

jdhoffa commented Dec 16, 2024

No worries at all! We never had any actual discussion around GH strategy, GH actions, anything like that, so most of the practices in this repo are mainly just organically intuited haha.

I think in general if a single action is failing and there's some exploration/ determination that it is unrelated to the PR, then it's ok to merge. But when all checks fail, it's usually an indication that something is broken by that PR.

Also surprised merging wasn't blocked ?

Re: I purposefully had not turned on "require status checks to pass before merging" on the branch protections. This is because, in my experience, there are often cases where GH actions fail and the failures are unrelated to the PR (maybe they are related to a change in the GH runner system itself, etc.). In general, I tried to opt for the more lenient/ less restrictive approach so people don't feel discouraged from contributing, but happy to change that depending on what folks prefer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

📝 DESCRIPTION gains authors
3 participants