Skip to content

Conversation

a1012112796
Copy link
Member

  • Add push commits history comment on PR time-line
  • Add notify by email and ui of this comment type also

view:
jh1


jh2


jh3

email
jf1


jf2

@6543
Copy link
Member

6543 commented Apr 21, 2020

@a1012112796 didn't looked at the code jet ... so question: Do a ForcePush delete old comments to commits who are gone now?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 21, 2020
@a1012112796
Copy link
Member Author

@a1012112796 didn't looked at the code jet ... so question: Do a ForcePush delete old comments to commits who are gone now?

I haven't do it now. first I have no idea now, Then I think keep old commits comment is usefull if you want find out previous commit that you has delted.
I will fix code lint and do some ui change this night.

@lunny lunny added type/feature Completely new functionality. Can only be merged if feature freeze is not active. pr/wip This PR is not ready for review labels Apr 21, 2020
@a1012112796 a1012112796 marked this pull request as draft April 21, 2020 16:42
@a1012112796
Copy link
Member Author

The reason why this PR is marked as draft again is that I found the force-push judgment is wrong and I need to find other strategies. Thanks.

@6543
Copy link
Member

6543 commented Apr 21, 2020

take your time :)

@a1012112796 a1012112796 marked this pull request as ready for review April 23, 2020 00:07
@guillep2k guillep2k removed the pr/wip This PR is not ready for review label Apr 23, 2020
Copy link
Member

@guillep2k guillep2k left a comment

Choose a reason for hiding this comment

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

I think this requires some integration tests.

@a1012112796
Copy link
Member Author

@guillep2k How about this way? ebde1d3

@a1012112796
Copy link
Member Author

I would actually prefer that json would be used to content so there would be no need for such parsing

@lafriks But I have no ideal about how to transform a commit into a json struct, It have many different type value and many sub struct .... 😢

// Commit represents a git commit.
type Commit struct {
Branch string // Branch this commit belongs to
Tree
ID SHA1 // The ID of this commit object
Author *Signature
Committer *Signature
CommitMessage string
Signature *CommitGPGSignature
Parents []SHA1 // SHA1 strings
submoduleCache *ObjectCache
}

Need help about json, Thanks

@lafriks lafriks modified the milestones: 1.12.0, 1.13.0 May 16, 2020
@lafriks
Copy link
Member

lafriks commented May 17, 2020

What I meant about json it to have comment Content field value saved in json using structure like:

type PushActionContent struct {
    IsForcePush bool
    Commits     []string
}

this way IsForcePush would not need to be saved to database and no migration would be needed

@a1012112796 a1012112796 marked this pull request as ready for review May 19, 2020 15:46
@guillep2k
Copy link
Member

@lafriks your review is required. 😁

@guillep2k
Copy link
Member

Ping LG-TM

@6543
Copy link
Member

6543 commented May 21, 2020

sadly I found a "bug" #11536

mrsdizzie added a commit to mrsdizzie/gitea that referenced this pull request Jun 12, 2020
Fix: template: issue/default:20:17: executing "issue/default" at <.Comment.Type>: nil pointer evaluating interface {}.Type

Introduced in go-gitea#11167
lafriks pushed a commit that referenced this pull request Jun 12, 2020
* Fix nil pointer in default issue mail template

Fix: template: issue/default:20:17: executing "issue/default" at <.Comment.Type>: nil pointer evaluating interface {}.Type

Introduced in #11167

* another one

Co-authored-by: zeripath <[email protected]>
@lafriks lafriks added the type/changelog Adds the changelog for a new Gitea version label Jun 12, 2020
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Add push commits history comment on PR time-line
* Add notify by email and ui of this comment type also

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

* Add migrations for IsForcePush
* fix wrong force-push judgement
* Apply suggestions from code review
* Remove commit number check
* add own notify fun
* fix some typo

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

* fix lint

* fix style again, I forgot something before

* Change email notify way

* fix api

* add number check if It's force-push

* Add repo commit link fuction
remove unnecessary check
skip show push commits comment which not have commits alive

* Update issue_comment.go

* Apply suggestions from code review

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

* Apply suggestions from code review

* fix ui view

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

* fix height

* remove unnecessary style define

* simplify GetBranchName

* Apply suggestions from code review

* save commit ids and isForce push by json
* simplify GetBranchName

* fix bug

Co-authored-by: guillep2k <[email protected]>
Co-authored-by: mrsdizzie <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: silverwind <[email protected]>
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Fix nil pointer in default issue mail template

Fix: template: issue/default:20:17: executing "issue/default" at <.Comment.Type>: nil pointer evaluating interface {}.Type

Introduced in go-gitea#11167

* another one

Co-authored-by: zeripath <[email protected]>
@a1012112796 a1012112796 deleted the commits_comment branch September 12, 2020 02:24
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. 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.

10 participants