Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add compare api #11187

Closed
wants to merge 150 commits into from
Closed

Add compare api #11187

wants to merge 150 commits into from

Conversation

Madongming
Copy link

Add compare api:
Respond to requests with api https://host:prot/api/v1/repos/:username/:reponame/git/compare/base...head
For the response structure, please refer to the file modules/structs/repo_compare.go

6543 and others added 30 commits January 8, 2020 17:32
Signed-off-by: jolheiser <john.olheiser@gmail.com>

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>
* Add HTML URL to API Issues

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* Swagger

Signed-off-by: jolheiser <john.olheiser@gmail.com>

Co-authored-by: Lauris BH <lauris@nix.lv>

Co-authored-by: Lauris BH <lauris@nix.lv>
* Fix nil reference

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* Tighten

Signed-off-by: jolheiser <john.olheiser@gmail.com>
Co-authored-by: zeripath <art27@cantab.net>
)

* Restore IsPasswordSet previous value

* Update models/user.go

Co-authored-by: Lauris BH <lauris@nix.lv>
Fomantic expects all tabs to have a target element with content as
defined by the data-tab attribute. All our usage of the tab module seems
to use <a> element tabs that link to new pages so these content elements
are never present and fomantic complains about that in the console with
an "Activated tab cannot be found" error. This silences that error.
* fix

* fix options

* add TEST

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>
…-gitea#9725)

* Delay printing hook statuses until after 1 second

* Move to a 5s delay, wrapped writer structure and add config

* Update cmd/hook.go

* Apply suggestions from code review

* Update cmd/hook.go

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>
* Move Errored PRs out of StatusChecking (go-gitea#9675)

* Set Errored PRs out of StatusChecking

* Ensure that api status is correctly set too

* Update models/pull.go

Co-Authored-By: John Olheiser <42128690+jolheiser@users.noreply.github.com>

Co-authored-by: John Olheiser <42128690+jolheiser@users.noreply.github.com>

* Update services/pull/check.go

Co-authored-by: John Olheiser <42128690+jolheiser@users.noreply.github.com>
webpack polyfills did not work because useBuiltIns: 'entry' expects a
explicit core-js import. Changed it to 'usage' which does not require
these explicit imports and polyfills based on browserslist.

As a result, the built index.js now went from 128kB to 192kB.

Ref: https://babeljs.io/docs/en/babel-preset-env#usebuiltins
…) (go-gitea#9764)

* Fix missing updated time on migrated issues and comments

* Fix testing and missing updated on migrating pullrequest

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>

Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>

Co-authored-by: John Olheiser <42128690+jolheiser@users.noreply.github.com>
Co-authored-by: zeripath <art27@cantab.net>
Signed-off-by: jolheiser <john.olheiser@gmail.com>

Co-authored-by: zeripath <art27@cantab.net>

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>
* backport state of 2020-01-14

* Apply suggestions from code review

Co-Authored-By: 6543 <6543@obermui.de>

Co-authored-by: zeripath <art27@cantab.net>
* backport korean 20-20-01-15 | fix go-gitea#9761

* update state 14:43:10 UTC
* Fix push-to-create

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* Check URL path and service

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* Send dummy payload on receive-pack GET

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* The space was actually a NUL byte

Signed-off-by: jolheiser <john.olheiser@gmail.com>

* Use real bare repo instead of manufactured payload

Signed-off-by: jolheiser <john.olheiser@gmail.com>

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: techknowlogick <matti@mdranta.net>

Co-authored-by: techknowlogick <matti@mdranta.net>
zeripath and others added 4 commits March 30, 2020 15:23
…#10883)

* Protect against NPEs in notifications list (go-gitea#10879)

Unfortunately there appears to be potential race with notifications
being set before the associated issue has been committed.

This PR adds protection in to the notifications list to log any failures
and remove these notifications from the display.

References go-gitea#10815 - and prevents the panic but does not completely fix
this.

Signed-off-by: Andrew Thornton <art27@cantab.net>

* add log import

* Update models/notification.go

Co-Authored-By: Lauris BH <lauris@nix.lv>

Co-authored-by: Lauris BH <lauris@nix.lv>
…10904)

* Fix milestones too many SQL variables bug

* Fix test

* Don't display repositories with no milestone and fix tests

* Remove unused code and add some comments
* Only update merge_base if not already merged

Fix go-gitea#10766

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Prevent race in transfer pull request

* Update services/pull/pull.go
* output of changelog

* Apply suggestions from code review

* Update CHANGELOG.md

Co-authored-by: zeripath <art27@cantab.net>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 23, 2020
@jolheiser jolheiser added modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed type/enhancement An improvement of existing functionality labels Apr 23, 2020
return nil
}

func toCompareDiff(ctx *context.APIContext, compareDiffFile *gitdiff.DiffFile) (*api.DiffFile, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type convertions expecialy for API should go into modules/convert/....go

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method of organization is modeled after the file routers/api/v1/repo/commits.go, and it is not just a conversion structure, but also other logic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convertion logic also should went to convert ... make it reusable :)

@6543
Copy link
Member

6543 commented Apr 25, 2020

and it would be nice if you add some tests 👍


// GetCompare get a comparison of the two versions via shaes / branches / tags
func GetCompare(ctx *context.APIContext) {
// swagger:operation GET /repos/{owner}/{repo}/git/compare/{<base sha | branch | tag>}...{[<head repo>:]<sha | branch | tag>} comparison of the two versions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason to put it under /git/ ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because its data source is the git diff command. so..., Because his data source is the git diff command, just my thoughts :>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a closer look soon

@6543
Copy link
Member

6543 commented Apr 25, 2020

Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pleace make it more similar to githubs api - so other projects can easely port there software to gitea
& make it reusable!

Diff *Diff `json:"diff"`
BaseCommitID string `json:"base_commit_id"`
HeadCommitID string `json:"head_commit_id"`
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pleace add url, html_url, permalink_url, diff_url and patch_url as described in https://developer.github.com/v3/repos/commits/#compare-two-commits

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and also "status" would be helpfull

Commits []*Commit `json:"commits"`
CommitCount int `json:"commit_count"`
Diff *Diff `json:"diff"`
BaseCommitID string `json:"base_commit_id"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
BaseCommitID string `json:"base_commit_id"`
BaseCommit *Commit `json:"base_commit"`

and return a full Commit object

CommitCount int `json:"commit_count"`
Diff *Diff `json:"diff"`
BaseCommitID string `json:"base_commit_id"`
HeadCommitID string `json:"head_commit_id"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
HeadCommitID string `json:"head_commit_id"`
HeadCommit *Commit `json:"merge_base_commit"`

... as above


// Compare information.
type Compare struct {
Title string `json:"title"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Title is not needed: it will be reprecented by the url filds and the status is represented by total_commits & ahead_by+behind_by

Suggested change
Title string `json:"title"`

Title string `json:"title"`
Commits []*Commit `json:"commits"`
CommitCount int `json:"commit_count"`
Diff *Diff `json:"diff"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pleace split diff into files and diff_stats <- diff_stats is a custom field we can use to add stuff like TotalAddition and so on
files contain array of DiffFile

}

// DiffFile represents a file diff.
type DiffFile struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pleace make it the same type as github does -> I will need this in #10933 and #10921 too ...
If you like to add extra infos github does not profide this is fine but with two limitations: the new fileds should not colide with github onec and info should not be profided elswhere (only if this would cause to mouch new api calls ...)

@@ -0,0 +1,68 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename this file to repo_diff.go since it will be reused by other api endpoints

@@ -817,6 +817,7 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Get("/trees/:sha", context.RepoRef(), repo.GetTree)
m.Get("/blobs/:sha", context.RepoRef(), repo.GetBlob)
m.Get("/tags/:sha", context.RepoRef(), repo.GetTag)
m.Get("/compare/*", context.ReferencesGitRepo(false), repo.GetCompare)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move out of git group

@6543
Copy link
Member

6543 commented Apr 29, 2020

I will start on #10933 and #10921 witch will use similar struct types so pleace make it reusable!

@6543
Copy link
Member

6543 commented May 5, 2020

@Madongming I think you messed up things with an rebase or something like this :(

@6543
Copy link
Member

6543 commented May 5, 2020

If I guess you rebased your branch to release/v1.11 but you have to rebase it to master!!

@6543
Copy link
Member

6543 commented Jun 28, 2020

@Madongming whats the status of this?

@Madongming Madongming closed this Jul 26, 2020
@Madongming Madongming deleted the add-compare-api branch July 26, 2020 08:19
@Bowser1704
Copy link

@Madongming What's the situation now?

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.