Skip to content
This repository was archived by the owner on Sep 14, 2019. It is now read-only.

Conversation

jwsloan
Copy link
Contributor

@jwsloan jwsloan commented Nov 14, 2018

Prior to this change, all top-level keys of a config file would be
replaced entirely by the incoming config. For some apps, this may
be sufficient. However, some apps have complex settings, and the
extend feature is much more useful if you can be more granular about
what you are overriding.

With this change, you can keep as much or as little of the base config
as you'd like.

closes #21

Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

I like it 👍

Prior to this change, all top-level keys of a config file would be
replaced entirely by the incoming config. For some apps, this may
be sufficient. However, some apps have complex settings, and the
extend feature is much more useful if you can be more granular about
what you are overriding.

With this change, you can keep as much or as little of the base config
as you'd like.

closes probot#21
This matches probot/probot, and will hopefully pass the build.
@jwsloan
Copy link
Contributor Author

jwsloan commented Nov 14, 2018

As soon as this is complete, I'll be able to finish repository-settings/app#105 . That will make the Settings app (especially Protected Branches) considerably more useful and easier to use.

@travi
Copy link
Member

travi commented Nov 14, 2018

this will be great! i've been hoping for similar for things even as simple as labels, so i could define a list at the org (.github repo) level and add repo specific labels rather than needing to include the full list and letting them get out of sync.

@gr2m
Copy link
Contributor

gr2m commented Nov 14, 2018

deepmerge also merges arrays: https://runkit.com/gr2m/5beca2c2d45bc90012f63204. I don’t see a problem myself right now, but worth mentioning.

RunKit notebooks are interactive javascript playgrounds connected to a complete node environment right in your browser. Every npm module pre-installed.

@jwsloan
Copy link
Contributor Author

jwsloan commented Nov 15, 2018

@gr2m, that is actually one of the features I'm counting on. It allows you to pass a function to use when merging arrays, and I'm going to use it for branch merging in the Protected Branches plugin for the Settings app.

@jwsloan
Copy link
Contributor Author

jwsloan commented Nov 15, 2018

I considered adding a few different merge functions to this repo that someone could choose to use. Maybe one that overwrites the existing array, one that merges elements based on their index, and one that takes a predicate function like (TargetElement, SourceElement) -> Boolean, and merges any for which the function returns true.

@ahmadnassri
Copy link

LOVELY! thank you!

@jwsloan
Copy link
Contributor Author

jwsloan commented Nov 16, 2018

Thanks for getting this up to date, @hiimbex.

I'm curious if ya'll have considered switching to a rebase workflow instead of merging master into feature branches? In my experience, rebase along with only using fast-forward merges to master keeps master's history super clean.

@gr2m gr2m merged commit b11a23d into probot:master Nov 16, 2018
@gr2m
Copy link
Contributor

gr2m commented Nov 16, 2018

@jwsloan I agree with the rebase, I do it on all my repos. I use squash-merge, too. And I automate releases using semantic-release. We had that on Probot for a while but removed it after once a major version was released accidentally. I hope to get back to automating things, once semantic-release has a review step :)

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

Successfully merging this pull request may close these issues.

usage of object.assign renders the utility of this library limited
5 participants