Added enhancement Sync settings after installing the integration #74

knrt10 · 2018-03-19T09:53:54Z

This is reference to issue #1

Added Functionalities:

When a user has a predefined .github folder and they install the probot/settings app after that, then 2 functionalities are added

1.) If .github folder has settings.yml file then it will be automatically synced.

2.) If no settings.yml file is found in .github then the bot opens a PR with some default settings in settings.yml file.

3.) When PR is merged bot automatically delete the branch created by bot.

jwsloan · 2018-03-21T16:54:43Z

Thanks for working on this! Cool idea!
A concern I have... If I don't like every default setting this provides, then the PR is of no use, correct? What if it actually grabbed the current settings for that repo, and put in a PR creating a matching settings.yml file?

knrt10 · 2018-03-21T16:56:24Z

@jwsloan Yea this is nice idea. I will implement this and update the PR.

jwsloan · 2018-03-21T17:14:20Z

Now that I think about it some more, I think this will need to involve creating importers for each of the current plugins. The ones that exist now could be moved into an exporters file. Then you could look through each setting as it is, and pass those settings to the appropriate importer similar to the way the current plugins work from the settings file.

knrt10 · 2018-03-21T17:54:26Z

@jwsloan I implemented that

A concern I have... If I don't like every default setting this provides, then the PR is of no use, correct? What if it actually grabbed the current settings for that repo, and put in a PR creating a matching settings.yml file?

But I did'nt get this part

Now that I think about it some more, I think this will need to involve creating importers for each of the current plugins. The ones that exist now could be moved into an exporters file. Then you could look through each setting as it is, and pass those settings to the appropriate importer similar to the way the current plugins work from the settings file.

Would you please explain?
Thanks in advance.

jwsloan · 2018-03-22T12:13:07Z

@knrt10
The current functionality can be thought of as exporting the settings from the yml file to the actual repo settings. It has been implemented as separate plugins corresponding to API endpoints. This could have been done as one large file that updates all the things, but that was not the design decision. Each plugin only knows about one area to export settings for.

Hopefully that first part makes sense.

The new functionality this issue describes could be looked at the same way, only in reverse. Each end point that is represented in the settings file would have a specific importer js file. Of course, that would make actually writing out the yml file a bit interesting.

I could be totally wrong. And terrible at explaining it. Does it make any more sense now?

patcon · 2018-04-05T03:04:22Z

It seems like this might be working as submitted 🎉

I totally get where you're coming from @jwsloan, but in the interest of supporting a first-time contributor in getting to a successful merge (woooo party yeah!), could we move that import/export architecture enhancement into a new ticket? I feel like you've provided an awesome description that really tees it up for the next person, if we were to copy that over?

jwsloan · 2018-04-05T12:03:43Z

That's a good call, @patcon. It would be great if we could get #76 merged first, and then include the branch protection settings in this. It could lock down master, required Code Owner approval by default.

aryannarora · 2018-04-06T23:58:06Z

@knrt10 Great PR!! 🎉
One question though, why did we have to create our own encode function?

EDIT: Sorry, if it's a rookie one. 😆

patcon · 2018-04-07T01:16:53Z

TWINS! Had the same thought, but felt like I should research before asking :) But that was prob my ego speaking..!

knrt10 · 2018-04-07T03:30:19Z

@aryannarora @patcon Thanks. Whenever we want to create a github file using API our file contents should be base64 encoded as per API specifications and also when we receive contents of a repo its encoded so we need to decode it, and I could not find any packages so wrote the function for both encoding and decoding. This PR is using the encoding part and #77 is using both. Hope this helps. ✌️ .

JasonEtco · 2018-04-07T04:23:57Z

index.js

+                              '    permission: push'
+
+                const base64 = require('./encode')
+                const encodedString = base64.encode(string)


@knrt10 instead of using some custom created Base64 encoding, you can use Node.JS's built-in Buffer global:

const encodedString = Buffer.from(string).toString('base64')

Ok let me update both the changes

@knrt10 i was asking about this only.

@aryannarora finally changed it.

JasonEtco · 2018-04-07T04:25:30Z

index.js

+                const getRepo = await context.github.repos.get({owner: owner, repo: repoName})
+                await context.github.gitdata.createReference({owner: owner, repo: repoName, ref: 'refs/heads/probot', sha: sha})
+                // setting the template of file
+                const string = 'repository: \n' +


Can you please move this into a separate file? It'd also be good to use a template literal to avoid all of the \n characters. In the separate file, you can do:

module.exports = getRepo => ` name: ${getRepo.data.name} `

Ooooo... I am not too proud to admit that I just learned a thing. Thanks! 🎉

knrt10 · 2018-04-07T08:23:09Z

@JasonEtco changed the files as you asked. Please have a look.

itaditya · 2018-04-19T04:06:27Z

@knrt10 I checked that you are listening to installation_repositories.added event for syncing the .github/settings.yml file. However this event doesn't trigger when you install the app for the first time.

So if a user has no .github/settings.yml present and installs the app for the first time on his account, then no PR will be made as of now.

knrt10 · 2018-04-19T04:08:27Z

@itaditya Thanks. Let me update that too.

itaditya · 2018-04-19T04:13:29Z

for reference you can take a look at https://github.com/itaditya/probot-bootstrap/blob/master/index.js#L14 and https://github.com/itaditya/probot-bootstrap/blob/master/lib/onboard.js#L7

itaditya

Listen to installation event also

stale · 2018-07-21T04:51:40Z

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

knrt10 · 2018-07-21T06:48:25Z

yep still relevant

nesl247 · 2018-08-07T19:35:35Z

Any update on this?

stale · 2018-11-05T20:13:35Z

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

knrt10 · 2018-11-06T08:35:58Z

yep relevant

stale · 2019-02-04T11:46:16Z

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

knrt10 · 2019-02-04T13:00:43Z

Not at all relevant.

patcon · 2019-02-04T20:25:04Z

Just to check in @knrt10 (I'm way out of the loop on the settings plugin, though have recommended it around) -- is this actually irrelevant, or was there frustration involved in getting it merged?

I really appreciating what it added way back when I bug-tested it, so am confused why it stayed in limbo :)

knrt10 added 2 commits March 19, 2018 15:19

added enchancement sync settings after install

6db6698

added enchancement sync settings after install

82ee56d

knrt10 changed the title ~~Added enahancement Sync settings after installing the integration~~ Added enanhacement Sync settings after installing the integration Mar 19, 2018

knrt10 changed the title ~~Added enanhacement Sync settings after installing the integration~~ Added enhancement Sync settings after installing the integration Mar 19, 2018

create repo according to present repo settings

5edc089

JasonEtco suggested changes Apr 7, 2018

View reviewed changes

patcon mentioned this pull request Apr 7, 2018

Implement import/export architecture #81

Closed

corrected the reference

61d3d76

knrt10 added 2 commits April 13, 2018 17:12

delete branch when PR is merged

9c75dd9

Create a folder automatically

cf68cfd

itaditya suggested changes Apr 19, 2018

View reviewed changes

knrt10 added 3 commits April 19, 2018 11:17

Fixed Bugs

e90cc27

Reducing index.js

818df33

adding module

674e607

stale bot added the wontfix label Jul 21, 2018

stale bot removed the wontfix label Jul 21, 2018

stale bot added the wontfix label Nov 5, 2018

stale bot removed the wontfix label Nov 6, 2018

stale bot added the wontfix label Feb 4, 2019

knrt10 closed this Feb 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added enhancement Sync settings after installing the integration #74

Added enhancement Sync settings after installing the integration #74

knrt10 commented Mar 19, 2018 •

edited

Loading

jwsloan commented Mar 21, 2018

knrt10 commented Mar 21, 2018

jwsloan commented Mar 21, 2018

knrt10 commented Mar 21, 2018

jwsloan commented Mar 22, 2018

patcon commented Apr 5, 2018 •

edited

Loading

jwsloan commented Apr 5, 2018

aryannarora commented Apr 6, 2018 •

edited

Loading

patcon commented Apr 7, 2018 •

edited

Loading

knrt10 commented Apr 7, 2018 •

edited

Loading

JasonEtco Apr 7, 2018 •

edited

Loading

knrt10 Apr 7, 2018

aryannarora Apr 7, 2018

knrt10 Apr 7, 2018

JasonEtco Apr 7, 2018 •

edited

Loading

patcon Apr 7, 2018

knrt10 commented Apr 7, 2018

itaditya commented Apr 19, 2018

knrt10 commented Apr 19, 2018

itaditya commented Apr 19, 2018 •

edited

Loading

itaditya left a comment

stale bot commented Jul 21, 2018

knrt10 commented Jul 21, 2018

nesl247 commented Aug 7, 2018

stale bot commented Nov 5, 2018

knrt10 commented Nov 6, 2018

stale bot commented Feb 4, 2019

knrt10 commented Feb 4, 2019

patcon commented Feb 4, 2019 •

edited

Loading

Added enhancement Sync settings after installing the integration #74

Added enhancement Sync settings after installing the integration #74

Conversation

knrt10 commented Mar 19, 2018 • edited Loading

jwsloan commented Mar 21, 2018

knrt10 commented Mar 21, 2018

jwsloan commented Mar 21, 2018

knrt10 commented Mar 21, 2018

jwsloan commented Mar 22, 2018

patcon commented Apr 5, 2018 • edited Loading

jwsloan commented Apr 5, 2018

aryannarora commented Apr 6, 2018 • edited Loading

patcon commented Apr 7, 2018 • edited Loading

knrt10 commented Apr 7, 2018 • edited Loading

JasonEtco Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

knrt10 Apr 7, 2018

Choose a reason for hiding this comment

aryannarora Apr 7, 2018

Choose a reason for hiding this comment

knrt10 Apr 7, 2018

Choose a reason for hiding this comment

JasonEtco Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

patcon Apr 7, 2018

Choose a reason for hiding this comment

knrt10 commented Apr 7, 2018

itaditya commented Apr 19, 2018

knrt10 commented Apr 19, 2018

itaditya commented Apr 19, 2018 • edited Loading

itaditya left a comment

Choose a reason for hiding this comment

stale bot commented Jul 21, 2018

knrt10 commented Jul 21, 2018

nesl247 commented Aug 7, 2018

stale bot commented Nov 5, 2018

knrt10 commented Nov 6, 2018

stale bot commented Feb 4, 2019

knrt10 commented Feb 4, 2019

patcon commented Feb 4, 2019 • edited Loading

knrt10 commented Mar 19, 2018 •

edited

Loading

patcon commented Apr 5, 2018 •

edited

Loading

aryannarora commented Apr 6, 2018 •

edited

Loading

patcon commented Apr 7, 2018 •

edited

Loading

knrt10 commented Apr 7, 2018 •

edited

Loading

JasonEtco Apr 7, 2018 •

edited

Loading

JasonEtco Apr 7, 2018 •

edited

Loading

itaditya commented Apr 19, 2018 •

edited

Loading

patcon commented Feb 4, 2019 •

edited

Loading