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

sync PR's comments #595

Merged
merged 1 commit into from
Jun 12, 2017
Merged

sync PR's comments #595

merged 1 commit into from
Jun 12, 2017

Conversation

samoht
Copy link
Member

@samoht samoht commented Jun 8, 2017

Feature

The goal is to expose PR's comments in the DataKit API, so DataKit CI can use it to implement something similar to https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin /cc @dave-tucker who needs this the LinuxKit's CI.

An example on how to use this:

$ jbuilder exec -- datakit-bridge-github -d git:///tmp/foo -r samoht/test
Starting datakit-bridge-github %%VERSION%% (*:rw)...
Connecting to git:///tmp/foo [github-metadata].
[ ... wait 1s and CTRL^C ... ] 

$ cat /tmp/foo/samoht/test/pr/32/comments/0/body
This is a test!

$ tree /tmp/foo/samoht/test/pr/32/
/tmp/foo/samoht/test/pr/32/
├── base
├── comments
│   ├── 0
│   │   ├── body
│   │   ├── id
│   │   └── user
│   └── 1
│       ├── body
│       ├── id
│       └── user
├── head
├── owner
├── state
└── title

Status

One-way sync (from GitHub to DataKit) are working.

Limitatations

DataKit-to-GitHub sync is not yet working but all the infrastructure is in place.

We need to be careful with the added extra startup time, as more files/directories means more 9p messages which might add quite a bit of lags on startup. I'll make some measurement before enabling that feature, but 9p is supposed to disappear soon and we don't restart the CI server very often, so that's not a bloquer.

Feedback is welcome.

@samoht samoht force-pushed the comments branch 4 times, most recently from d869e98 to a81e467 Compare June 9, 2017 16:10
@samoht samoht changed the title [WIP] sync PR's comments sync PR's comments Jun 9, 2017
Comments are stored as directories into `user/repo/prs/<id>/comments/`. The PR's
body is stored as `user/repo/prs/<id>/comments/0` and subsequent comments are
stored under `1/`, `2/`, etc in the order in which they have been published.

Every comment directory has the following files:

- `id`: comment ID. Useful if we want to be able to edit them later;
- `user`: user who created the comment.
- `body`: comment's body

Currently, it's only a a one-way sync (eg. from GitHub into DataKit) but that's
already useful on its own.

Signed-off-by: Thomas Gazagnaire <thomas@gazagnaire.org>
@samoht samoht merged commit 66b0526 into moby:master Jun 12, 2017
@samoht samoht deleted the comments branch June 12, 2017 13:15
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 21, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 23, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 23, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 23, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
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.

None yet

1 participant