Re: [Dev] Bake on Gitlab

[Aaron Wolf]

On 08/10/2015 04:38 PM, Bryan Richter wrote:
> On Mon, Aug 10, 2015 at 11:29:36AM +0300, Nikita Karetnikov wrote:
>> FYI, I renamed the project to snowdrift-ci because I'm no longer
>> planning to use Bake:
>>
>> https://github.com/ndmitchell/bake/issues/15#issuecomment-129343416
> 
> I have a lot of concerns about this. I am concerned about starting yet
> another in-house project that solves problems that have existing
> solutions. (FLO solutions, even.) I also haven't seen any concrete
> reasons why Bake can't be used.
> 
> Over all of this, though, I am concerned that we are spreading
> ourselves way too thin. Github + Travis does exactly... *exactly*...
> what we need. Can we please make one tiny concession to our present
> reality and switch to using it?
> 
> Will anyone on this list be forced to stop contributing if we make
> Github our primary source location?
> 
> 

As long as we *accept* code from other sources, nobody will be
disenfranchised by our emphasis on GitHub, but as long as we don't
*require* GitHub, I'm confused about how it would work. If we can have a
process that is realistic for doing Travis CI stuff on GitHub even when
code comes in from git.gnu.io, then why can't we just do that anyway
while continuing to emphasize git.gnu.io? For that matter, did we ever
actually consider GitLab CI?

I also don't understand what's going on with Bake, nor do I personally
yet fully appreciate the need to prioritize CI, but I don't have
experience to judge.

FWIW, GitLab.com itself is preferable to GitHub for various reasons (no
proprietary JS!), so if hosting *there* was required for using GitLab
CI, it would be an option to consider *if* GitLab CI would be a real answer.

I also don't understand why CI is so complex. From Nikita's wording, it
sounds like he actually thinks it'd be pretty easy to just get it
working his way… but I also am super wary about overly-optimistic
attempts to take on everything and end up with way more hassle than
anticipated…

I do *not* fundamentally oppose focusing on GitHub and Travis, it just
seems crazy that it would be the only reasonable solution. Personally,
I'd be okay with just pushing forward without CI, but again, I don't
understand what we're missing.

Overall, it's quite frustrating to keep running into these walls that
aren't the core focus, but I do think at the *least* its valuable to at
least *see* what the possibilities are for doing things ideally before
we give up, but maybe we've done that at this point… not sure…

-- 
Aaron Wolf Snowdrift.coop 
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Bryan Richter]

On Mon, Aug 10, 2015 at 11:29:36AM +0300, Nikita Karetnikov wrote:
> FYI, I renamed the project to snowdrift-ci because I'm no longer
> planning to use Bake:
> 
> https://github.com/ndmitchell/bake/issues/15#issuecomment-129343416

I have a lot of concerns about this. I am concerned about starting yet
another in-house project that solves problems that have existing
solutions. (FLO solutions, even.) I also haven't seen any concrete
reasons why Bake can't be used.

Over all of this, though, I am concerned that we are spreading
ourselves way too thin. Github + Travis does exactly... *exactly*...
what we need. Can we please make one tiny concession to our present
reality and switch to using it?

Will anyone on this list be forced to stop contributing if we make
Github our primary source location?


signature.asc
Description: Digital signature
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Nikita Karetnikov]

FYI, I renamed the project to snowdrift-ci because I'm no longer
planning to use Bake:

https://github.com/ndmitchell/bake/issues/15#issuecomment-129343416
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Bryan Richter]

On Tue, Aug 04, 2015 at 07:08:35PM +, Stephen Paul Weber wrote:
> >The two primary mechanical tasks are (1) merging the patch
> >with master, (2) compiling the merged code, (3) testing it, and (4)
> >reporting on any errors in the first three steps. The goal for bake,
> >then, is to automate these tasks.
> 
> Any maybe running analysis tools like hlint?

I support that eventual goal. We gotta make everything pass first
though. :)

If anyone wants to help in this, we could get started by having a
whitelist of files known to pass hlint. We could then automate linting
those files, and fill out the list until it encompasses the whole
project.

I suggest this because there are 132 Haskell files in the project
right now, and making them all pass at once would probably be too
hasty. Better to ramp up and make sure we're comfortable about what
suggestions we enforce or ignore.


signature.asc
Description: Digital signature
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Stephen Paul Weber]

The two primary mechanical tasks are (1) merging the patch
with master, (2) compiling the merged code, (3) testing it, and (4)
reporting on any errors in the first three steps. The goal for bake,
then, is to automate these tasks.


Any maybe running analysis tools like hlint?


I think to satisfy these goals, we *don't* want bake pushing to
master. That should be a manual step. What bake should do instead is
report, by adding comments or tags to the merge request.


Yes.  Definitely.
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Nikita Karetnikov]

>> It's not clear how to use web hooks to trigger Bake when someone
>> pushes to a branch that's already associated with a merge request.
> 
> No message is sent in that case? :( The docs specifically say that the
> merge request event is "triggered when a new merge request is created,
> an existing merge request was updated/merged/closed or a commit is
> added in the source branch." Are the docs wrong?

No, I just can't read.  Though, I haven't tested it myself.
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Bryan Richter]

On Mon, Aug 03, 2015 at 11:20:33PM +0300, Nikita Karetnikov wrote:
> Ah, one more thing: should this be a separate project with its own
> .cabal file?  I think so, but it never hurts to ask.

Definitely. I don't even think it makes sense to put it in the same
stack.yaml, as devs don't need it for developing on Snowdrift.


signature.asc
Description: Digital signature
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Bryan Richter]

On Mon, Aug 03, 2015 at 11:14:01PM +0300, Nikita Karetnikov wrote:
> I've got GitLab working (the same version that git.gnu.io uses) inside a
> VM, so now I can finally reply.
> 
> > Yes, that would be possible, but I think it would not meet the goal of
> > automating the mechanical aspects of a code review. I think we want to
> > test, then review, then merge, in that order.
> 
> Okay, though, I think the other approach would be easier.  Anyway, here
> is a more detailed workflow:
> 
> 1. Someone opens a merge request.
> 
> 2. The merge request hook sends json to Bake:
> 
>
> https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/web_hooks/web_hooks.md#merge-request-events
> 
>(The URI can be provided in the GitLab UI.)
> 
>The "state" attribute must be set to "opened" in this case.  (The
>other ones are "reopened" and "closed".)
> 
> 3. Bake uses the "object_attributes" fields to fetch the source
>branch.
> 
> 4. Bake runs the tests.
> 
>(Question: should Bake test after merging the source to target or
>not?)

It should test after the merge, and report merge failures as well as
test failures.

> 5. Bake posts a report to a merge request using the API:
> 
>http://doc.gitlab.com/ce/api/merge_requests.html#post-comment-to-mr
> 
> It's not clear how to use web hooks to trigger Bake when someone
> pushes to a branch that's already associated with a merge request.

No message is sent in that case? :( The docs specifically say that the
merge request event is "triggered when a new merge request is created,
an existing merge request was updated/merged/closed or a commit is
added in the source branch." Are the docs wrong?

> Folks would definitely want to correct things (that's the point of
> having a review process after all).

Precisely.

> We could probably work around that by using the "comment on merge
> request" hook.  Something like: "Bake it!"  But I don't really like this
> solution since it's error-prone.

Definitely.


signature.asc
Description: Digital signature
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Nikita Karetnikov]

Ah, one more thing: should this be a separate project with its own
.cabal file?  I think so, but it never hurts to ask.
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Nikita Karetnikov]

I've got GitLab working (the same version that git.gnu.io uses) inside a
VM, so now I can finally reply.

> Yes, that would be possible, but I think it would not meet the goal of
> automating the mechanical aspects of a code review. I think we want to
> test, then review, then merge, in that order.

Okay, though, I think the other approach would be easier.  Anyway, here
is a more detailed workflow:

1. Someone opens a merge request.

2. The merge request hook sends json to Bake:

   
https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/web_hooks/web_hooks.md#merge-request-events

   (The URI can be provided in the GitLab UI.)

   The "state" attribute must be set to "opened" in this case.  (The
   other ones are "reopened" and "closed".)

3. Bake uses the "object_attributes" fields to fetch the source
   branch.

4. Bake runs the tests.

   (Question: should Bake test after merging the source to target or
not?)

5. Bake posts a report to a merge request using the API:

   http://doc.gitlab.com/ce/api/merge_requests.html#post-comment-to-mr

It's not clear how to use web hooks to trigger Bake when someone
pushes to a branch that's already associated with a merge request.
Folks would definitely want to correct things (that's the point of
having a review process after all).

We could probably work around that by using the "comment on merge
request" hook.  Something like: "Bake it!"  But I don't really like this
solution since it's error-prone.

I only know how to do it properly when people are allowed to push
since there is a web hook for pushes.  Ideas?
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Bryan Richter]

On Thu, Jul 30, 2015 at 07:38:45PM +0300, Nikita Karetnikov wrote:
> So I'm looking into Bake integration with Gitlab.  Here's the rough
> plan:
> 
> 1. Setup web hooks as described here:
>
> https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/web_hooks/web_hooks.md
> 
> 2. Parse that on the Bake side.
> 
> 3. Give Bake push access to master.

Let's take a step back and start with the goals and purpose. Then we
can be sure the plan fulfills them. Here's my proposal:

Gitlab merge requests are a tool we use for code review. That will
always be a manual process, by definition. The review team manually
reads the code, interacts with the contributor, and ultimately accepts
or rejects the patch.

There are some mechanical steps to a good code review, however,
and CI can perform those automatically. The purpose of doing these
things automatically is to provide quick, automatic feedback to a
contributor, as well as to save time and effort from the review
team.  The two primary mechanical tasks are (1) merging the patch
with master, (2) compiling the merged code, (3) testing it, and (4)
reporting on any errors in the first three steps. The goal for bake,
then, is to automate these tasks.

---

I think to satisfy these goals, we *don't* want bake pushing to
master. That should be a manual step. What bake should do instead is
report, by adding comments or tags to the merge request.

> It'd be ideal if Bake could test merge requests, but there are
> some challenges.  There's only one object type for merge requests,
> which can be in multiple states: open, updated, merged, closed.  So
> there doesn't seem to be a way to clearly indicate: "test this!"

Whether bake does too many builds (e.g. for closed or merged requests)
is not a dealbreaker, but obviously it would be nice to inspect the
web hook payload and only run for open or updated requests.

I take it there is no way to control bake's behavior? It sounds like
that migt be necessary for both how Gitlab interact with bake, and how
bake interacts with Gitlab. If it is not possible, I can think of a
couple possible solutions:

1.  Add hooks on bake's side!
2.  Add a proxy between Gitlab and bake.
3.  Others?

> ...
> Then, the workflow becomes: a user pushes to a feature branch on the
> main repo, Bake takes that as "ready for merge" and tests; merges if
> appropriate.  Is it possible with Gitlab?

Yes, that would be possible, but I think it would not meet the goal of
automating the mechanical aspects of a code review. I think we want to
test, then review, then merge, in that order.

I'm happy that you're working on this, by the way. I think it would be
a great addition to our workflow.


signature.asc
Description: Digital signature
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

[Dev] Bake on Gitlab

[Nikita Karetnikov]

So I'm looking into Bake integration with Gitlab.  Here's the rough
plan:

1. Setup web hooks as described here:
   
https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/web_hooks/web_hooks.md

2. Parse that on the Bake side.

3. Give Bake push access to master.

It'd be ideal if Bake could test merge requests, but there are some
challenges.  There's only one object type for merge requests, which can
be in multiple states: open, updated, merged, closed.  So there doesn't
seem to be a way to clearly indicate: "test this!"  Bake could test and
merge sequentially, but that might result in weird history.  There could
be a delay, say, 24 hours.  But this is not how Bake is intended to be
used.  Paraphrasing Neil: "Bake is for semi-trusted teams where people
can make mistakes but are not malicious."  Which means giving access to
all regular contributors.  Then, the workflow becomes: a user pushes to
a feature branch on the main repo, Bake takes that as "ready for merge"
and tests; merges if appropriate.  Is it possible with Gitlab?

Right now, I'm just trying to run Gitlab locally and hoping to do some
simple testing.  That's all for now.
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev

Re: [Dev] Bake on Gitlab

[Aaron Wolf]

On 07/30/2015 12:38 PM, Nikita Karetnikov wrote:
> So I'm looking into Bake integration with Gitlab.  Here's the rough
> plan:
> 
> 1. Setup web hooks as described here:
>
> https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/web_hooks/web_hooks.md
> 
> 2. Parse that on the Bake side.
> 
> 3. Give Bake push access to master.
> 
> It'd be ideal if Bake could test merge requests, but there are some
> challenges.  There's only one object type for merge requests, which can
> be in multiple states: open, updated, merged, closed.  So there doesn't
> seem to be a way to clearly indicate: "test this!"  Bake could test and
> merge sequentially, but that might result in weird history.  There could
> be a delay, say, 24 hours.  But this is not how Bake is intended to be
> used.  Paraphrasing Neil: "Bake is for semi-trusted teams where people
> can make mistakes but are not malicious."  Which means giving access to
> all regular contributors.  Then, the workflow becomes: a user pushes to
> a feature branch on the main repo, Bake takes that as "ready for merge"
> and tests; merges if appropriate.  Is it possible with Gitlab?
> 

Yes, we can give bake push permission for master and we could extend the
permissions to more people to push to branches on the main repo that
bake will watch.

I'm not sure the details, but perhaps we ask contributors to open MRs
that aren't MRs to master branch but to a bake-ready feature branch or
something like that. I could also imagine new devs opening traditional
MRs but devs that are trusted get permission to push to feature-branches
on the main repo…


> Right now, I'm just trying to run Gitlab locally and hoping to do some
> simple testing.  That's all for now.
> ___
> Dev mailing list
> Dev@lists.snowdrift.coop
> https://lists.snowdrift.coop/mailman/listinfo/dev
> 

-- 
Aaron Wolf Snowdrift.coop 
___
Dev mailing list
Dev@lists.snowdrift.coop
https://lists.snowdrift.coop/mailman/listinfo/dev