-
Notifications
You must be signed in to change notification settings - Fork 182
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
[Discussion] Work on new major release #38
Comments
I think this is a good undertaking, and I'm especially interested in updating the syntax, updating the node engine, and writing unit tests/incorporating CI. I don't have a whole lot of availability through the end of the year but will do what I can to see this through. Since Gatekeeper is a relatively small repo, I think this can still move forward. @kriswep how would you prioritize the steps here, and are there specific chunks you would particularly like to work on? |
@kriswep I would recommend leaving the Node engine version requirement unchanged. I changed it recently to I think the rest of your suggestions are great ideas, though. I don’t have nearly as much free time as I would like, but I could probably help out a little and do some code reviews. |
Maybe we could also add a linter to the build process to enforce certain standards for syntax after the refactoring is complete. For example, preventing the use of the |
@compumike08 I added linting to the list above, great point. Reason for updating node would be the native support for async/await. I think the difference between requiring node 6 and node 8 isn't that hard. People who are on node 6 probably can upgrade to 8 rather easily. Plus any users on heroku or the like would get the upgrade basically for free. @dereklieu I'd start with creating a Plus maybe add me as collaborator with write access to this project, so I could help with this stuff. 😄 |
@kriswep You make a good point about native I wouldn’t call the branch |
I thought of |
Oh, ok then.
… On Oct 23, 2017, at 3:45 PM, Christoph Benjamin Weber ***@***.***> wrote:
I thought of dev as a kind of consolidating branch, while we are working on different new features.
The named feature branches is what you would have in your working fork, right?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
So, what do you think about a target branch for the refactor? Would like to send a first PR for discussion. |
@kriswep @dereklieu Actually, I think the question of what to name the branch might be getting a bit ahead of ourselves. A decision should probably be made as to what Git workflow the project should follow. A good guide to several common Git workflows is available here. Personally, I would recommend the Forking Workflow for this project. This workflow has the advantage of not requiring the owner of Gatekeeper to give additional people write access to the repository. I would make one small change to the workflow from what is described in the link above, though. I recommend that instead of PR from forked repos being made against the original Gatekeeper repo’s What do you guys think? |
I agree with the forking workflow and I'm ok calling the working branch either The practice where I work is similar, and we keep a forever-running I'd also advocate to open specific refactor tasks as individual tickets so they can be more easily assigned and tagged to pull requests. @kriswep since you created the initial list, I'd love if you could also create the tickets. Other users can then comment on individual tickets and we can decide which ones to work on (some are no-brainers, some might take some discussion). To facilitate this I'm creating a As a last note, as Gatekeeper currently doesn't include a test suite and some of the changes we're proposing are quite invasive, I would advocate we develop a basic test suite first (or close to first) so that future work can include regression tests. I'm open to suggestions on frameworks, but prefer lightweight runners like https://github.com/substack/tape. |
Sounds good to me regarding the workflow. |
Hello, I think the correct way of working is creating a fork and using small feature-focused branches. That way changes can be increasingly incorporated into gatekeeper and several people can work on different topics at the same time. I can easily advance the following tasks. Note that instead of listing the tasks, I'll list the branches I would use:
Let me think what do you think about the proposed workflow. Regards |
Anyone want to say something about my proposal ? |
Thanks for chiming in. Sounds good to me, I guess every help would be appreciated. I‘d say, work on CI features and/or code refactoring would be most helpful |
Hey @danielo515 sorry for my late response. I think the proposed tasks are great. I have some preferences on linting but none particularly strongly held. If you have a lint rule-set suggestion, please put it forward in a ticket and we can go from there. And yes, please do fork and open small feature branches, and pr against the |
How is this going, all? Has development stopped? I stumbled upon this repo right before deciding to simply write my own implementation, but if it could be streamlined, I could possibly help with that some. |
@muziejus we implemented a few of the proposed items but are still in need of a linter and tests. If you are open to contributing those, or any of the other tasks outlined here, I'd be happy to work with you on them. |
OK. I'll get back to this in a bit, then. Thanks! |
I made a serverless solution with Netlify Functions if that helps anyone: https://github.com/cadbox1/github-oktokit-oauth-netlify |
First things first: I used gatekeeper in a little side project and it worked great for me.
But, looking at the source, the project starts to feel a little aged. This might turn potential users off and to newer solutions like micro-github or micro-auth.
Although they look like great work as well, I like gatekeeper's concept more, as it does not put the access token in the users query url.
So I propose working on a new major release/overhaul of gatekeeper.
Changes I have in mind (in no particular order):
What do you think about that? Happy to discuss this and other ideas.
I would also like to participate in this upgrade.
The text was updated successfully, but these errors were encountered: