update description about vendoring in CONTRIBUTING.md #18280

a1012112796 · 2022-01-15T10:16:07Z

TODO:

any other things about Remove golang vendored directory ?

follow go-gitea#18277 Signed-off-by: a1012112796 <[email protected]>

CONTRIBUTING.md

codecov-commenter · 2022-01-15T10:43:20Z

Codecov Report

❗ No coverage uploaded for pull request base (main@7dde39a). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main   #18280   +/-   ##
=======================================
  Coverage        ?   45.75%           
=======================================
  Files           ?      831           
  Lines           ?    92204           
  Branches        ?        0           
=======================================
  Hits            ?    42188           
  Misses          ?    43254           
  Partials        ?     6762

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7dde39a...3f7b3f5. Read the comment docs.

wxiaoguang · 2022-01-15T10:43:53Z

CONTRIBUTING.md

+Pull requests should only include `go.mod`, `go.sum` updates if they are part of
 the same change, be it a bugfix or a feature addition.


I can not quite understand the purpose of this description. Maybe it can be simplified. The Go Modules document already contains all information.

/cc @lunny @techknowlogick

Suggested change

Pull requests should only include `go.mod`, `go.sum` updates if they are part of

the same change, be it a bugfix or a feature addition.

I think the proposed change is correct and should remain (see below for the rationale).

The old text was talking about vendor directory. But now, that is it talking about?

if they are part of the same change: what is the same change ...
be it a bugfix or a feature addition what does the bug fix or feature addition refer to ....

wxiaoguang · 2022-01-15T10:45:38Z

CONTRIBUTING.md

+The `go.mod`, `go.sum` update needs to be justified as part of the PR description,
 and must be verified by the reviewers and/or merger to always reference
 an existing upstream commit.


If we use Go Modules, we do not need to check the upstream commit again. This check was required by the committed vendor directory in my mind.

The intent of this paragraph is to require a justification whenever a dependency is added or modified. I think the proposed change is correct. Adding a new dependency previously meant modifying vendor/, go.mod and go.sum. Requiring an explanation when vendor/ was modified implied requiring an explanation when go.mod and go.sum was modified. Now that vendor/ is removed, there is a need to rephrase the requirement.

Co-authored-by: wxiaoguang <[email protected]>

singuliere · 2022-01-15T11:05:35Z

CONTRIBUTING.md

+The `go.mod`, `go.sum` update needs to be justified as part of the PR description,
 and must be verified by the reviewers and/or merger to always reference
 an existing upstream commit.


The intent of this paragraph is to require a justification whenever a dependency is added or modified. I think the proposed change is correct. Adding a new dependency previously meant modifying vendor/, go.mod and go.sum. Requiring an explanation when vendor/ was modified implied requiring an explanation when go.mod and go.sum was modified. Now that vendor/ is removed, there is a need to rephrase the requirement.

singuliere · 2022-01-15T11:06:17Z

CONTRIBUTING.md

+Pull requests should only include `go.mod`, `go.sum` updates if they are part of
 the same change, be it a bugfix or a feature addition.


I think the proposed change is correct and should remain (see below for the rationale).

singuliere · 2022-01-15T11:08:10Z

The .gitignore file contains a /vendor line now obsolete. It is not strictly in scope for this PR but since you're looking for elements that became redundant after the removal of the vendor directory, here is one :-)

singuliere · 2022-01-15T11:11:35Z

And another one: the test-vendor target in Makefile. And grep -v /vendor/ in Makefile as well.

silverwind · 2022-01-15T16:23:47Z

This .gitattributes entry can be removed:

/vendor/** -text -eol linguist-vendored

Makefile

Signed-off-by: a1012112796 <[email protected]>

a1012112796 · 2022-01-16T12:34:41Z

wonder if build.go still usefull?

gitea/build.go

Lines 1 to 25 in 4d0a72a

    
           // Copyright 2020 The Gitea Authors. All rights reserved. 
        
           // Use of this source code is governed by a MIT-style 
        
           // license that can be found in the LICENSE file. 
        
           //go:build vendor 
        
           // +build vendor 
        
           package main 
        
           // Libraries that are included to vendor utilities used during build. 
        
           // These libraries will not be included in a normal compilation. 
        
           import ( 
        
           	// for embed 
        
           	_ "github.com/shurcooL/vfsgen" 
        
           	// for cover merge 
        
           	_ "golang.org/x/tools/cover" 
        
           	// for vet 
        
           	_ "code.gitea.io/gitea-vet" 
        
           	// for swagger 
        
           	_ "github.com/go-swagger/go-swagger/cmd/swagger" 
        
           )

.gitattributes

This reverts commit 4789e70.

silverwind · 2022-01-17T14:04:39Z

wonder if build.go still usefull?

I think it's probably obsolete, but this removal can also be done in another PR.

* giteaoffical/main: Restore propagation of ErrDependenciesLeft (go-gitea#18325) Fix PR comments UI (go-gitea#18323) Make the height of the editor in Review Box smaller (4 lines as GitHub) (go-gitea#18319) Fix commit links on compare page (go-gitea#18310) Update JS dependencies, remove eslint-plugin-github (go-gitea#18317) Add MirrorUpdated field to Repository API type (go-gitea#18267) replace satori/go.uuid with gofrs/uuid (go-gitea#18311) Place inline diff comment dialogs in the 4th column. (go-gitea#18321) Use indirect comparison when showing pull requests (go-gitea#18313) Prevent ambiguous column error in organizations page (go-gitea#18314) Correctly upload LFS files (go-gitea#18316) [skip ci] Updated translations via Crowdin update description about vendoring in CONTRIBUTING.md (go-gitea#18280) Fix CheckRepoStats and reuse it during migration (go-gitea#18264) Minor tweak to tag list (go-gitea#18295)

@silverwind

* update description about vendoring in CONTRIBUTING.md follow go-gitea#18277 Signed-off-by: a1012112796 <[email protected]> * Update CONTRIBUTING.md Co-authored-by: wxiaoguang <[email protected]> * fix and readd test-vendor step * remove vendor from .gitattributes @silverwind * simplify go mod check Signed-off-by: a1012112796 <[email protected]> * Revert "remove vendor from .gitattributes @silverwind" This reverts commit 4789e70. Co-authored-by: wxiaoguang <[email protected]> Co-authored-by: zeripath <[email protected]> Co-authored-by: 6543 <[email protected]>

update description about vendoring in CONTRIBUTING.md

dc9a3df

follow go-gitea#18277 Signed-off-by: a1012112796 <[email protected]>

wxiaoguang reviewed Jan 15, 2022

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 15, 2022

wxiaoguang reviewed Jan 15, 2022

View reviewed changes

Update CONTRIBUTING.md

8a6b1d0

Co-authored-by: wxiaoguang <[email protected]>

singuliere approved these changes Jan 15, 2022

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 15, 2022

a1012112796 added 2 commits January 15, 2022 19:25

Merge branch 'main' into remove_vendor_doc

620ee47

fix and readd test-vendor step

686e8de

zeripath approved these changes Jan 15, 2022

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 15, 2022

zeripath and others added 3 commits January 15, 2022 23:28

Merge branch 'main' into remove_vendor_doc

c91dcb9

remove vendor from .gitattributes @silverwind

4789e70

Merge branch 'main' into remove_vendor_doc

a287e33

wxiaoguang added type/docs This PR mainly updates/creates documentation skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Jan 16, 2022

wxiaoguang and others added 2 commits January 16, 2022 13:16

Merge branch 'main' into remove_vendor_doc

74d6f59

Merge branch 'main' into remove_vendor_doc

dd0cd45

silverwind reviewed Jan 16, 2022

View reviewed changes

Makefile Outdated Show resolved Hide resolved

simplify go mod check

6aef0e8

Signed-off-by: a1012112796 <[email protected]>

silverwind reviewed Jan 16, 2022

View reviewed changes

.gitattributes Outdated Show resolved Hide resolved

Merge branch 'main' into remove_vendor_doc

18c1abb

Revert "remove vendor from .gitattributes @silverwind"

d61ada4

This reverts commit 4789e70.

silverwind approved these changes Jan 17, 2022

View reviewed changes

Merge branch 'main' into remove_vendor_doc

3f7b3f5

techknowlogick merged commit 8581e2f into go-gitea:main Jan 17, 2022

a1012112796 deleted the remove_vendor_doc branch January 18, 2022 01:30

go-gitea locked and limited conversation to collaborators Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update description about vendoring in CONTRIBUTING.md #18280

update description about vendoring in CONTRIBUTING.md #18280

a1012112796 commented Jan 15, 2022

codecov-commenter commented Jan 15, 2022 •

edited

Loading

wxiaoguang Jan 15, 2022

a1012112796 Jan 15, 2022

singuliere Jan 15, 2022

wxiaoguang Jan 15, 2022

wxiaoguang Jan 15, 2022

singuliere Jan 15, 2022

singuliere Jan 15, 2022

singuliere Jan 15, 2022

singuliere commented Jan 15, 2022 •

edited

Loading

singuliere commented Jan 15, 2022 •

edited

Loading

silverwind commented Jan 15, 2022

a1012112796 commented Jan 16, 2022

silverwind commented Jan 17, 2022

		Pull requests should only include `go.mod`, `go.sum` updates if they are part of
		the same change, be it a bugfix or a feature addition.

update description about vendoring in CONTRIBUTING.md #18280

update description about vendoring in CONTRIBUTING.md #18280

Conversation

a1012112796 commented Jan 15, 2022

codecov-commenter commented Jan 15, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

singuliere commented Jan 15, 2022 • edited Loading

singuliere commented Jan 15, 2022 • edited Loading

silverwind commented Jan 15, 2022

a1012112796 commented Jan 16, 2022

silverwind commented Jan 17, 2022

codecov-commenter commented Jan 15, 2022 •

edited

Loading

singuliere commented Jan 15, 2022 •

edited

Loading

singuliere commented Jan 15, 2022 •

edited

Loading