search

thoughtbot / factory_bot

A library for setting up Ruby objects as test data.
https://thoughtbot.com
MIT License
7.89k stars 2.6k forks source link

Update README again #1630

Closed stefannibrasil closed 3 months ago

stefannibrasil commented 3 months ago

There's a rule for when a branch is merged for factory_bot. I tried removing [ci-skip] from the original workflow to trigger all CI actions. It didn't work, still got the same error:

remote: error: GH006: Protected branch update failed for refs/heads/main.
remote: error: Required status check "Run standard" is expected.
To https://github.com/thoughtbot/factory_bot.git
 ! [remote rejected] main -> main (protected branch hook declined)

Gonna try going through the PR workflow to see if the rule gets happy.

stefannibrasil commented 3 months ago

Trying it now.

stefannibrasil commented 3 months ago

@mike-burns sorry to bother you but running into some issues with the dynamic README workflow. I opened a PR and standard was green. Not sure why I still get this error:

remote: error: GH006: Protected branch update failed for refs/heads/main.        
remote: error: Required status check "Run standard" is expected.        
To https://github.com/thoughtbot/factory_bot.git
 ! [remote rejected] main -> main (protected branch hook declined)
error: failed to push some refs to '***github.com/thoughtbot/factory_bot.git'

That's because we have this branch rule:

Choose which [status checks](https://docs.github.com/rest/commits/statuses) must pass before branches can be merged into a branch that matches this rule. When enabled, commits must first be pushed to another branch, then merged or pushed directly to a branch that matches this rule after status checks have passed.

Standard was passing when I opened this PR. Do you know what else it needs to be changed for the dynamic readme action to run successfully? 🤔 I also don't want to pollute the git history. Will do some research and try on another test repo but let me know if you have any ideas. Thanks!

stefannibrasil commented 3 months ago

Ahhhhh, got the answer: so the problem is that the dynamic README pushes to main and this repo has a rule against that. I'm curious to know if you're open to considering removing that rule? Any strong cases for keeping it? @mike-burns If you prefer keeping that, I can revert the dynamic README until we figure out how to handle this scenario.

mike-burns commented 3 months ago

The only rule this repo has for pushes against main is that CI must pass. It looks like Standard is failing (or maybe I'm reading the error wrong and CI simply isn't running). Is there a way to do this whole process while making sure CI passes?

stefannibrasil commented 3 months ago

Is there a way to do this whole process while making sure CI passes?

I'm gonna explore this option and let you know 👍

stefannibrasil commented 3 months ago

Okay, put up a PR to handle this case: https://github.com/thoughtbot/templates/pull/31 Once that's merged, I will update this repo.