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.

Sign up for GitHub

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

Jump to bottom

Reviews and Pull Request comments #2

Merged
merged 18 commits into from
May 13, 2018
Merged

Reviews and Pull Request comments #2

merged 18 commits into from
May 13, 2018

Conversation

jonasfranz
Copy link

@jonasfranz jonasfranz commented May 6, 2018 •
edited
Loading

This should contribute to go-gitea#3748.

  • Resolve conflicts
  • Add review model
  • Add comment types

This is WIP.

Reviews

Reviews should bundle all code comments created for the initial review. All code comments created for this review are linked to the review model.

Comment types

CommentTypeCode: A comment at a line of a code of a PR (UI not implemented)
CommentTypeReview: Notifies the participants about the review. (UI not implemented)

TODO

  • Render code comments on the right place
  • Add support for comments on both sides
  • Show code comments in the "conversation" after a review got submitted
  • Show single code comment in the "conversation"
  • Remove dummy data from "code comment form" (line, path, side)
  • Add support for "Add single comments"
  • Order/Separate code comments by thread/review
  • Blocked by Add line blame go-gitea/git#116

@jonasfranz
Add Review
623a9f6 
Add IssueComment types

Signed-off-by: Jonas Franz <[email protected]>

(cherry picked from commit 2b4daab)
Signed-off-by: Jonas Franz <[email protected]>
jonasfranz added 4 commits May 6, 2018 23:32
@jonasfranz
Replace ReviewComment with Content
aeb0577 
Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Add load functions
2c18552 
Add ReviewID to findComments

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Add create review comment implementation
9544c46 
Add migration for review
Other small changes

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Simplified create and find functions for review
0f772d1 
Signed-off-by: Jonas Franz <[email protected]>
lafriks
lafriks reviewed May 10, 2018
View reviewed changes
models/review.go Outdated
ReviewTypeComment
// ReviewTypeReject gives feedback blocking merge
ReviewTypeReject
// ReviewTypePending is a review which is not published yet
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be better that this would be moved as first

Copy link
Author

Choose a reason for hiding this comment

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

@lafriks Done

jonasfranz added 5 commits May 10, 2018 21:43
@jonasfranz
Moved "Pending" to first position
4ad563d 
Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Add GetCurrentReview to simplify fetching current review
17af2d1 
Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Preview for listing comments
75b7d9b 
Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Move new comment form to its own file
e252d3b 
Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Implement Review form
3e5f3c3 
Show Review comments on comment stream

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Copy link
Author

jonasfranz commented May 11, 2018 •
edited
Loading

I've implemented the submit review form by implementing review comments:
review comments
I am not really contented with the icons. UI needs to be improved but won't in this PR.

Previously I've added a way of rendering comments in pull files code.
This is really buggy and I need some help with that @lafriks.
At the moment it only works for the splitted diff view and takes up way to much space. In addition it is only shown on the right side every time.
comments on code

@jonasfranz jonasfranz mentioned this pull request May 11, 2018
Merged
23 tasks
jonasfranz added 3 commits May 11, 2018 13:40
@jonasfranz
Add support for single comments
61cc134 
Showing buttons in context

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Add pending tag to pending review comments
36d6631 
Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Add unit tests for Review
9c6bb4b 
Signed-off-by: Jonas Franz <[email protected]>
lafriks
lafriks reviewed May 11, 2018
View reviewed changes
models/issue_comment.go Outdated
return nil, err
}
for _, comment := range comments {
if err = comment.loadReview(e); err != nil && !IsErrReviewNotExist(err) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be remade to load reviews at once for all comments

Copy link
Author

Choose a reason for hiding this comment

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

@lafriks Done

jonasfranz added 3 commits May 11, 2018 20:28
@jonasfranz
Fetch all review ids at once
bc93592 
Add unit tests

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
gofmt
58fb672 
Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Improved comment rendering in "Files" view by adding Comments to Diff…
066086c 
…Line

Signed-off-by: Jonas Franz <[email protected]>
@jonasfranz
Copy link
Author

@lafriks Now it works also for the unified view but adding comments will result in adding comments at the wrong place. This happens because the javascript part is not ready yet.

@jonasfranz jonasfranz mentioned this pull request May 13, 2018
Merged
@jonasfranz
Copy link
Author

jonasfranz commented May 13, 2018 •
edited
Loading

Comments get invalidated now after the commented LOC get changed by a push to the head branch. That is implemented via git blame. => Blocked by go-gitea/git#116

@appleboy
Copy link

@JonasFranzDEV merged go-gitea/git#116

@lafriks lafriks merged commit 2f46613 into lafriks-fork:feat/approval May 13, 2018
lafriks added a commit that referenced this pull request May 17, 2020
@aqtrans @techknowlogick
@6543 @lafriks @lunny
Add support for migrating from Gitlab (go-gitea#9084)
5c092eb 
* First stab at a Gitlab migrations interface.

* Modify JS to show migration for Gitlab

* Properly strip out #gitlab tag from repo name

* Working Gitlab migrations!
Still need to figure out how to hide tokens/etc from showing up in opts.CloneAddr

* Try #2 at trying to hide credentials.
CloneAddr was being used as OriginalURL.
Now passing OriginalURL through from the form and saving it.

* Add go-gitlab dependency

* Vendor go-gitlab

* Use gitlab.BasicAuthClient
Correct CloneURL.
This should be functioning!
Previous commits fixed "Migrated from"
from including the migration credentials.

* Replaced repoPath with repoID globally.
RepoID is grabbed in NewGitlabDownloader

* Logging touchup

* Properly set private repo status.
Properly set milestone deadline time.
Consistently use Gitlab username for 'Name'.

* Add go-gitlab vendor cache

* Fix PR migrations:
- Count of issues is kept to set a non-conflicting PR.ID
- Bool is used to tell whether to fetch Issue or PR comments

* Ensure merged PRs are closed and set with the proper time

* Remove copyright and some commented code

* Rip out '#gitlab' based self-hosted Gitlab support

* Hide given credentials for migrated repos.

CloneAddr was being saved as OriginalURL.

Now passing OriginalURL through from the form and
saving it in it's place

* Use asset.URL directly, no point in parsing.
Opened PRs should fall through to false.

* Fix importing Milestones.
Allow importing using Personal Tokens or anonymous access.

* Fix Gitlab Milestone migration if DueDate isn't set

* Empty Milestone due dates properly return nil, not zero time

* Add GITLAB_READ_TOKEN to drone unit-test step

* Add working gitlab_test.go.
A Personal Access Token, given in env variable GITLAB_READ_TOKEN
is required to run the test.

* Fix linting issues

* Add modified JS files

* Remove pre-build JS files

* Only merged PRs are marged as merged/closed

* Test topics

* Skip test if gitlab is inaccessible

* Grab personal token from username, not password.
Matches Github migration implementation

* Add SetContext() to GitlabDownloader.

* Checking Updated field in Issues.

* Actually fetch Issue Updated time from Gitlab

* Add Gitlab migration GetReviews() stub

* Fix Patch and Clone URLs

* check Updated too

* fix mod

* make vendor with go1.14

Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
lafriks pushed a commit that referenced this pull request Aug 5, 2020
@6543
Kanban board suggestions (#2)
338e8bb 
* 6543 suggestions

* fix
lafriks pushed a commit that referenced this pull request Apr 25, 2022
@KN4CK3R @wxiaoguang @tboerger
Add Package Registry (go-gitea#16510)
1d33234 
* Added package store settings.

* Added models.

* Added generic package registry.

* Added tests.

* Added NuGet package registry.

* Moved service index to api file.

* Added NPM package registry.

* Added Maven package registry.

* Added PyPI package registry.

* Summary is deprecated.

* Changed npm name.

* Sanitize project url.

* Allow only scoped packages.

* Added user interface.

* Changed method name.

* Added missing migration file.

* Set page info.

* Added documentation.

* Added documentation links.

* Fixed wrong error message.

* Lint template files.

* Fixed merge errors.

* Fixed unit test storage path.

* Switch to json module.

* Added suggestions.

* Added package webhook.

* Add package api.

* Fixed swagger file.

* Fixed enum and comments.

* Fixed NuGet pagination.

* Print test names.

* Added api tests.

* Fixed access level.

* Fix User unmarshal.

* Added RubyGems package registry.

* Fix lint.

* Implemented io.Writer.

* Added support for sha256/sha512 checksum files.

* Improved maven-metadata.xml support.

* Added support for symbol package uploads.

* Added tests.

* Added overview docs.

* Added npm dependencies and keywords.

* Added no-packages information.

* Display file size.

* Display asset count.

* Fixed filter alignment.

* Added package icons.

* Formatted instructions.

* Allow anonymous package downloads.

* Fixed comments.

* Fixed postgres test.

* Moved file.

* Moved models to models/packages.

* Use correct error response format per client.

* Use simpler search form.

* Fixed IsProd.

* Restructured data model.

* Prevent empty filename.

* Fix swagger.

* Implemented user/org registry.

* Implemented UI.

* Use GetUserByIDCtx.

* Use table for dependencies.

* make svg

* Added support for unscoped npm packages.

* Add support for npm dist tags.

* Added tests for npm tags.

* Unlink packages if repository gets deleted.

* Prevent user/org delete if a packages exist.

* Use package unlink in repository service.

* Added support for composer packages.

* Restructured package docs.

* Added missing tests.

* Fixed generic content page.

* Fixed docs.

* Fixed swagger.

* Added missing type.

* Fixed ambiguous column.

* Organize content store by sha256 hash.

* Added admin package management.

* Added support for sorting.

* Add support for multiple identical versions/files.

* Added missing repository unlink.

* Added file properties.

* make fmt

* lint

* Added Conan package registry.

* Updated docs.

* Unify package names.

* Added swagger enum.

* Use longer TEXT column type.

* Removed version composite key.

* Merged package and container registry.

* Removed index.

* Use dedicated package router.

* Moved files to new location.

* Updated docs.

* Fixed JOIN order.

* Fixed GROUP BY statement.

* Fixed GROUP BY #2.

* Added symbol server support.

* Added more tests.

* Set NOT NULL.

* Added setting to disable package registries.

* Moved auth into service.

* refactor

* Use ctx everywhere.

* Added package cleanup task.

* Changed packages path.

* Added container registry.

* Refactoring

* Updated comparison.

* Fix swagger.

* Fixed table order.

* Use token auth for npm routes.

* Enabled ReverseProxy auth.

* Added packages link for orgs.

* Fixed anonymous org access.

* Enable copy button for setup instructions.

* Merge error

* Added suggestions.

* Fixed merge.

* Handle "generic".

* Added link for TODO.

* Added suggestions.

* Changed temporary buffer filename.

* Added suggestions.

* Apply suggestions from code review

Co-authored-by: Thomas Boerger <[email protected]>

* Update docs/content/doc/packages/nuget.en-us.md

Co-authored-by: wxiaoguang <[email protected]>
Co-authored-by: Thomas Boerger <[email protected]>
lafriks pushed a commit that referenced this pull request Jul 2, 2024
@silverwind
Upgrade golang.org/x/net to v0.24.0 (go-gitea#30283)
9550404 
Result of `go get -u golang.org/x/net; make tidy`.

This is related to the following vulncheck warning:
```
There are 2 vulnerabilities in modules that you require that are
neither imported nor called. You may not need to take any action.
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.

Vulnerability #1: GO-2024-2687
    HTTP/2 CONTINUATION flood in net/http
  More info: https://pkg.go.dev/vuln/GO-2024-2687
  Module: golang.org/x/net
    Found in: golang.org/x/[email protected]
    Fixed in: golang.org/x/[email protected]

Vulnerability #2: GO-2022-0470
    No access control in github.com/blevesearch/bleve and bleve/v2
  More info: https://pkg.go.dev/vuln/GO-2022-0470
  Module: github.com/blevesearch/bleve/v2
    Found in: github.com/blevesearch/bleve/[email protected]
    Fixed in: N/A
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lafriks lafriks lafriks left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jonasfranz @appleboy @lafriks