Permalink
Browse files

Problem: pull requests are not preserved in SIT

The fact of a pull request on GitHub, or the subsequent
conversation do not get preserved in SIT, resulting a loss
of information.

Solution: designate a simple "merge request" workflow
based on MergeRequested record type.
  • Loading branch information...
yrashk committed Feb 5, 2018
1 parent d280dfe commit a65efb539fe6d61d5d82f64f2e28c4956c5e5550
Showing with 105 additions and 12 deletions.
  1. +22 −6 CONTRIBUTING.md
  2. +4 −0 PULL_REQUEST_TEMPLATE.md
  3. +6 −2 README.md
  4. +23 −0 doc/dict/.type/MergeRequested.md
  5. +50 −4 sit-core/src/reducers/core.rs
@@ -1,11 +1,16 @@
**This project does not accept GitHub Pull Requests**. This is done intentionally,
as it allows us to maintain the entire history of submitted patches in SIT itself. Any
opened Pull Requests will be closed. The contributors will be asked to use our
own merge request procedure (see below).
Our goal is to encourage frictionless contributions to the project. In order to
achieve that, we use Unprotocols' [C4 process](https://rfc.unprotocols.org/spec:1/C4)
as an inspiration. Please read it, it will answer a lot of questions. Our goal is to
merge pull requests as quickly as possible and make new stable releases regularly.
merge patches as quickly as possible and make new stable releases regularly.
In a nutshell, this means:
* We merge pull requests rapidly (try!)
* We merge patches rapidly (try!)
* We are open to diverse ideas
* We prefer code now over consensus later
@@ -46,7 +51,18 @@ Solution: explanation of the solution to the problem. Could
be longer than one line.
```
Typically, patches are expected to be submitted over a pull request, but
if your circumstances preclude you from doing so, you can reach out to
any maintainer privately by e-mail or any other available means and
send the patch that way.
# Preparing a merge request
Once you have a branch (BRANCH) with your patch ready:
1. Create a new issue: `sit issue` and take a note of the generated ID.
2. Generate patches: `rm -rf git && git format-patch $(git merge-base --fork-point master BRANCH)..BRANCH -o git` (cleaning `git` assures there are no leftover patches)
2. Edit temporary `text` file to supply the one-line summary (such as `Problem: ...`).
It is important to name the file `text` and not something else
3. Take ID from the first step and run `sit record -t SummaryChanged ID text`
4. Edit temporary `text` file to prepare details.
Provide detailed information for your patch so that others can fully
understand it. It is a good etiquette to have one or a few paragraphs.
4. Take ID from the first step and run `sit record -t DetailsChanged,MergeRequested ID text git/*.patch`
5. Refer to [this instruction](https://github.com/sit-it/sit#send-it-to-upstream) to send the merge request to the upstream.
@@ -0,0 +1,4 @@
# Pull Requests Are Not Supported
Please refer to [contribution guidelines](https://github.com/sit-it/sit/blob/master/CONTRIBUTING.md)
for the merge request workflow.
@@ -205,6 +205,10 @@ This way, pushing out, will be as nice as `git push issues <branch>`
You will get all issue updates when you fetch this git repository.
### Preparing a merge request
Please refer to [CONTRIBUTING](https://github.com/sit-it/sit/blob/master/CONTRIBUTING.md#preparing-a-merge-request] for the instruction.
## Overview
<center>
@@ -355,11 +359,11 @@ contributors.
Our goal is to encourage frictionless contributions to the project. In order to
achieve that, we use Unprotocols' [C4 process](https://rfc.unprotocols.org/spec:1/C4)
as an inspiration. Please read it, it will answer a lot of questions. Our goal is to
merge pull requests as quickly as possible and make new stable releases regularly.
merge patches as quickly as possible and make new stable releases regularly.
In a nutshell, this means:
* We merge pull requests rapidly (try!)
* We merge patches rapidly (try!)
* We are open to diverse ideas
* We prefer code now over consensus later
@@ -0,0 +1,23 @@
# MergeRequested
Indicates that the record includes a patchset to
be merged.
## Files
Since SIT is supposed to be SCM-agnostic (or even work with one), it supports
different ways to describe patchsets. Typically, only one way will be used
per record (typically, corresponding to the SCM currently used). In situations
when more than one method is used they all have to be *equivalent* patchsets.
### `git/*.patch`
Patches produced by `git format-patch`
### `patch/*.diff`
Diffs produced by `diff`
## State Effect
Appends `merge_requests` field (array) with the encoded record hash.
@@ -147,16 +147,48 @@ impl<R: Record + RecordExt> Reducer for CommentedReducer<R> {
}
}
/// Reduces Commented type
pub struct MergeRequestedReducer<R: Record>(PhantomData<R>);
impl<R: Record> MergeRequestedReducer<R> {
pub fn new() -> Self {
MergeRequestedReducer(PhantomData)
}
}
impl<R: Record + RecordExt> Reducer for MergeRequestedReducer<R> {
type State = Map<String, JsonValue>;
type Item = R;
fn reduce(&self, mut state: Self::State, item: &Self::Item) -> Self::State {
state.entry("merge_requests").or_insert(JsonValue::Array(vec![]));
if item.has_type("MergeRequested") {
// scope it to unborrow `requests` before it is returned
{
let requests = state.get_mut("merge_requests").unwrap();
let hash = item.encoded_hash();
requests.as_array_mut().unwrap().push(JsonValue::String(hash.as_ref().into()));
}
state
} else {
state
}
}
}
/// Combines Closed, SummaryChanged, DetailsChanged, Commented, MergeRequested reducers
pub struct BasicIssueReducer<R: Record>(ChainedReducer<MergeRequestedReducer<R>,
ChainedReducer<CommentedReducer<R>, ChainedReducer< ChainedReducer<IssueClosureReducer<R>, IssueSummaryReducer<R>>,
IssueDetailsReducer<R>>>>);
/// Combines Closed, SummaryChanged, DetailsChanged reducers
pub struct BasicIssueReducer<R: Record>(ChainedReducer<CommentedReducer<R>, ChainedReducer<ChainedReducer<IssueClosureReducer<R>, IssueSummaryReducer<R>>, IssueDetailsReducer<R>>>);
impl<R: Record> BasicIssueReducer<R> {
pub fn new() -> Self {
BasicIssueReducer(CommentedReducer::new()
BasicIssueReducer(MergeRequestedReducer::new().chain(CommentedReducer::new()
.chain(IssueClosureReducer::new()
.chain(IssueSummaryReducer::new())
.chain(IssueDetailsReducer::new())))
.chain(IssueDetailsReducer::new()))))
}
}
@@ -250,4 +282,18 @@ mod tests {
}
#[test]
fn merge_requested() {
let mut tmp = TempDir::new("sit").unwrap().into_path();
tmp.push(".sit");
let repo = Repository::new(tmp).unwrap();
let issue = repo.new_issue().unwrap();
let record = issue.new_record(vec![(".type/MergeRequested", &b""[..])].into_iter(), true).unwrap();
let state = issue.reduce_with_reducer(MergeRequestedReducer::new()).unwrap();
let requests = state.get("merge_requests").unwrap().as_array().unwrap();
assert_eq!(requests.len(), 1);
assert_eq!(requests[0], record.encoded_hash());
}
}

0 comments on commit a65efb5

Please sign in to comment.