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

Update repository config on all default branch commits #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pecigonzalo
Copy link

This changes the behavior of the bot to update the repo config on all commits
We want our org/.github/.github/settings.yml to apply to all repos by default, by filtering on only commits that update the .github/settings.yml files we are unable to do so.

Current behavior

  • To trigger an update of the config, we need to "modify" or fake a modification of the .github/settings.yml
  • Repos do NOT inherit by default from org/.github repo (as probot-config suggests for opt-in/opt-out scenarios)

Desired behavior

  • Any push to the default branch triggers the bot settings update
  • Inherit by default from org/.github repository
Notes

The org/.github repository config should NOT include the following sections

  name: repo-name
  description: description of repo
  homepage: https://example.github.io/
  topics: github, probot

Maybe we could even filter them on the bot

Related

@@ -6,15 +6,15 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => {
const payload = context.payload
const defaultBranch = payload.ref === 'refs/heads/' + payload.repository.default_branch

const config = await getConfig(context, 'settings.yml', {}, { arrayMerge: mergeArrayByName })
if (!defaultBranch) {
// Not the defualt branch, nothing to see here!
Copy link
Author

Choose a reason for hiding this comment

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

Im not sure how to print with debug logs in probot :( but this should be a debug log

})

if (defaultBranch && settingsModified) {
Copy link
Author

Choose a reason for hiding this comment

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

Moved the if for applying settings or not to the top, as if its not the case, there is no point on running the deepmerge

@pecigonzalo
Copy link
Author

pecigonzalo commented Apr 4, 2019

@stale
Copy link

stale bot commented Jul 3, 2019

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Jul 3, 2019
@pecigonzalo
Copy link
Author

Waiting for feedback Mr Bot

@stale stale bot removed the wontfix label Jul 4, 2019
@allejo
Copy link

allejo commented Sep 4, 2019

Not being able to sync the settings.yml file without a commit to it specifically is a pain point for my team as well. Not sure how I feel about triggering a sync after every commit because it seems excessive but I like this solution until something else can be implemented.

@stale
Copy link

stale bot commented Dec 3, 2019

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Dec 3, 2019
@mortenlj
Copy link

mortenlj commented Dec 3, 2019

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

Still relevant, blocked by project being dead?

@stale stale bot removed the wontfix label Dec 3, 2019
@travi
Copy link
Member

travi commented Dec 4, 2019

@mortenlj the project is not dead, but is now maintained by a few volunteers that are not working full time on this project. i'm sorry that there hasn't been feedback on this specific PR since I joined as a maintainer, but please consider that the maintainers are volunteering their time to keep the project moving forward rather than becoming dead, even if addressing some problems takes longer than we all would like.

While applying changes to account-level config is a problem that we want to solve, we want to proceed carefully since the possible solutions do come with tradeoffs. addressing this problem introduces a very significant behavioral change to how settings works currently. i'm at least wanting to make sure that i understand the system well before making a hasty change in this area. we are considering some other solutions, like #179, which could solve the same problem with a less expensive approach.

we understand that solving this need is something that will benefit most settings users, including the usage by the maintainers. i can assure you that the need is not going unnoticed.

@pecigonzalo
Copy link
Author

pecigonzalo commented Dec 4, 2019 via email

@mortenlj
Copy link

mortenlj commented Dec 4, 2019

@mortenlj the project is not dead, but is now maintained by a few volunteers that are not working full time on this project. i'm sorry that there hasn't been feedback on this specific PR since I joined as a maintainer, but please consider that the maintainers are volunteering their time to keep the project moving forward rather than becoming dead, even if addressing some problems takes longer than we all would like.

@travi I appreciate that it's a volunteer effort, and I really do value your work. My reason for asking if the project is dead is because in my view, one of the most important things to do when maintaining an open source project with too little time is to give potential new maintainers/contributors feedback when they open a PR. Since this PR has not received any feedback in 8 months, my thoughts were that there hasn't been anyone maintaining the project in 8 months.

If the project isn't dead, and progress is being made, then that's excellent news! I eagerly await a solution to the problem. I would have contributed myself, except I don't know up or down in a javascript codebase. 😄

@travi
Copy link
Member

travi commented Dec 6, 2019

it would be great then to mark this so it stops getting the stale every now and then, as it's really annoying to comment every X and just adds noise to the other people following this, including myself.

agreed on this point. i still need to understand how the bot is configured for this project, but i have been annoyed by this as well, including before i became a maintainer. i don't have direct control of this, so i may not be able to improve the situation immediately, but do hope to get some improvements figured out.

As you know, my time to update, research how this project works, etc, was
also volunteered and would have appreciated at least a comment on this as
it was opened 8 months ago aside from a bot.

understood and your contribution is appreciated. not that it helps much now, but i'll clarify that i was not a maintainer at the time. that doesn't excuse the lack of commenting since joining, but i am still working to catch up on outstanding issues and PRs, including figuring out some of the ones that were closed by stale bot that shouldn't have been. part of the reason for the delay is that solving this need is a pretty big change to overall behavior, so i've been trying to get my feet under myself with some smaller features first.

i do definitely want to solve this need though. i think i currently do lean in the direction of a solution along the lines of #179 so that the updates would be even more immediate, but only when the account-level config has actually been changed. i would love additional feedback if you have some on either approach.

@travi
Copy link
Member

travi commented Dec 6, 2019

My reason for asking if the project is dead

please be mindful that suggesting that a project is dead is more aggressive than just an innocent question. i recognize that this project has been quiet for a while, especially during the period of time when this PR was opened. however, it is pretty easy to to find recent activity with issues, PRs, and commits to understand that this project is not dead, even though our new maintainers, myself included, have not yet circled back to this PR.

one of the most important things to do when maintaining an open source project with too little time is to give potential new maintainers/contributors feedback when they open a PR.

unfortunately, i only became a maintainer recently, so there is an existing backlog of outstanding issues and PRs, some of which have wrongly been closed by the stale bot that we are still working to get caught up on. i do hope to get to the point where responsiveness is improved for new contributions, but available volunteer hours are limited.

i do understand the frustration, but also appreciate understanding that maintenance efforts do have limits, especially when trying to catch back up with outstanding desires.

@pecigonzalo
Copy link
Author

pecigonzalo commented Dec 6, 2019 via email

@mortenlj
Copy link

mortenlj commented Dec 6, 2019

please be mindful that suggesting that a project is dead is more aggressive than just an innocent question

My apologies if my comments come of as aggressive, that is certainly not my intention. Thanks for picking up the mantle, and just know that we are all eagerly awaiting any and all improvements you can make. 😄

@stale
Copy link

stale bot commented Mar 5, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Mar 5, 2020
@pecigonzalo
Copy link
Author

pecigonzalo commented Mar 6, 2020 via email

@stale stale bot removed the wontfix label Mar 6, 2020
@stale
Copy link

stale bot commented Jun 4, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Jun 4, 2020
@allejo
Copy link

allejo commented Jun 20, 2020

Bump. Still relevant

@stale stale bot removed the wontfix label Jun 20, 2020
@stale
Copy link

stale bot commented Sep 20, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@stale stale bot added the wontfix label Sep 20, 2020
@allejo
Copy link

allejo commented Sep 20, 2020

bad bot

@stale stale bot removed the wontfix label Sep 20, 2020
@rogerluan
Copy link

Ping @travi what do we need to merge this in? 🙇

At the moment, the alternative to this bot, https://github.com/github/safe-settings, is ahead of this one because it doesn't support this feature 😥

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.

5 participants