Serve .patch for pull requests #3305
Conversation
|
I don't know why the patch is served as type octect-stream instead of text/plain, ideas @bkcsoft ? |
tboerger
added
the
lgtm/need 2
routers/repo/pull.go
Outdated
| return | ||
| } | ||
|
|
||
| // Redirect elsewhere if it's not a pull request |
lafriks
added
the
type/feature
Codecov Report
@@ Coverage Diff @@
## master #3305 +/- ##
==========================================
- Coverage 35.08% 35.06% -0.02%
==========================================
Files 279 279
Lines 40526 40562 +36
==========================================
+ Hits 14220 14225 +5
- Misses 24206 24229 +23
- Partials 2100 2108 +8
Continue to review full report at Codecov.
|
tboerger
added
lgtm/need 1
|
@strk I would like this based on go-gitea/git#105 |
integrations/pull_create_test.go
Outdated
| // check .patch can be accessed and matches performed change | ||
| req = NewRequest(t, "GET", url+".patch") | ||
| resp = session.MakeRequest(t, req, http.StatusOK) | ||
| assert.Regexp(t, "\\+Hello, World \\(Edited\\)", resp.Body) |
There was a problem hiding this comment.
Why not `\+Hello, World \(Edited\)` (raw string)? Seems cleaner to me
integrations/pull_create_test.go
Outdated
| resp = session.MakeRequest(t, req, http.StatusOK) | ||
| assert.Regexp(t, "\\+Hello, World \\(Edited\\)", resp.Body) | ||
| assert.Regexp(t, "diff", resp.Body) | ||
| assert.Regexp(t, "Subject: \\[PATCH\\] Update 'README.md'", resp.Body) |
|
I didn't understand the suggestion of raw string, are you asking
why I'm using regexp instead of substring ?
|
|
@strk he was suggesting to use golang raw strings to not to have escape backslash characters in it |
Closes go-gitea#3259 Updates "git" module, for GetFormatPatch
|
I've updated to new |
tboerger
added
lgtm/done
|
@strk you should handle |
|
error handled with 8243e4f |
| return | ||
| } | ||
|
|
||
| _, err = io.Copy(ctx, patch) |
There was a problem hiding this comment.
@strk You need to set the approriate Content-Type before this :)
There was a problem hiding this comment.
The PR is merged, if you can handle to test Content-Type in the existing integration test that'd be very useful (Content-Type was automatically added in previous implementation via ServeContent)
Closes #3259