Merge branch 'master' into fix/3336

.github/workflows/bot.yml

@@ -14,6 +14,10 @@ on:
       - converted_to_draft
       - ready_for_review
 
+  # Watch for changes on PR reviews
+  pull_request_review:
+    types: [submitted, edited, dismissed]
+
   # Watch for changes on PR comment
   issue_comment:
     types: [created, edited, deleted]

CONTRIBUTING.md

@@ -469,6 +469,18 @@ Resources for idiomatic Go docs:
 - [godoc](https://go.dev/blog/godoc)
 - [Go Doc Comments](https://tip.golang.org/doc/comment)
 
+## Avoding Unhelpful Contributions
+
+While we welcome all contributions to the Gno project, it's important to ensure that your changes provide meaningful value or improve the quality of the codebase. Contributions that fail to meet these criteria may not be accepted. Examples of unhelpful contributions include (but not limited to):
+
+- Airdrop farming & karma farming: Making minimal, superficial changes, with the goal of becoming eligible for airdrops and GovDAO participation.
+- Incomplete submissions: Changes that lack adequate context, link to a related issue, documentation, or test coverage.
+
+Before submitting a pull request, ask yourself:
+- Does this change solve a specific problem or add clear value?
+- Is the implementation aligned with the gno.land's goals and style guide?
+- Have I tested my changes and included relevant documentation?
+
 ## Additional Notes
 
 ### Issue and Pull Request Labels
@@ -502,3 +514,4 @@ automatic label management.
 | info needed      | Issue is lacking information needed for resolving |
 | investigating    | Issue is still being investigated by the team     |
 | question         | Issue starts a discussion or raises a question    |
+

contribs/github-bot/internal/check/check.go

@@ -101,7 +101,8 @@ func processPRList(gh *client.GitHub, prs []*github.PullRequest) error {
 		go func(pr *github.PullRequest) {
 			defer wg.Done()
 			commentContent := CommentContent{}
-			commentContent.allSatisfied = true
+			commentContent.AutoAllSatisfied = true
+			commentContent.ManualAllSatisfied = true
 
 			// Iterate over all automatic rules in config.
 			for _, autoRule := range autoRules {
@@ -120,7 +121,7 @@ func processPRList(gh *client.GitHub, prs []*github.PullRequest) error {
 					thenDetails.SetValue(fmt.Sprintf("%s Requirement satisfied", utils.Success))
 					c.Satisfied = true
 				} else {
-					commentContent.allSatisfied = false
+					commentContent.AutoAllSatisfied = false
 				}
 
 				c.ConditionDetails = ifDetails.String()
@@ -160,8 +161,14 @@ func processPRList(gh *client.GitHub, prs []*github.PullRequest) error {
 					},
 				)
 
-				if checkedBy == "" {
-					commentContent.allSatisfied = false
+				// If this check is the special one, store its state in the dedicated var.
+				if manualRule.Description == config.ForceSkipDescription {
+					if checkedBy != "" {
+						commentContent.ForceSkip = true
+					}
+				} else if checkedBy == "" {
+					// Or if its a normal check, just verify if it was checked by someone.
+					commentContent.ManualAllSatisfied = false
 				}
 			}
 
@@ -224,9 +231,20 @@ func logResults(logger logger.Logger, prNum int, commentContent CommentContent)
 	}
 
 	logger.Infof("Conclusion:")
-	if commentContent.allSatisfied {
-		logger.Infof("%s All requirements are satisfied\n", utils.Success)
+
+	if commentContent.AutoAllSatisfied {
+		logger.Infof("%s All automated checks are satisfied", utils.Success)
+	} else {
+		logger.Infof("%s Some automated checks are not satisfied", utils.Fail)
+	}
+
+	if commentContent.ManualAllSatisfied {
+		logger.Infof("%s All manual checks are satisfied\n", utils.Success)
 	} else {
-		logger.Infof("%s Not all requirements are satisfied\n", utils.Fail)
+		logger.Infof("%s Some manual checks are not satisfied\n", utils.Fail)
+	}
+
+	if commentContent.ForceSkip {
+		logger.Infof("%s Bot checks are force skipped\n", utils.Success)
 	}
 }

contribs/github-bot/internal/check/comment.go

@@ -24,9 +24,9 @@ var errTriggeredByBot = errors.New("event triggered by bot")
 // Compile regex only once.
 var (
 	// Regex for capturing the entire line of a manual check.
-	manualCheckLine = regexp.MustCompile(`(?m:^-\s\[([ xX])\]\s+(.+?)\s*(\(checked by @(\w+)\))?$)`)
+	manualCheckLine = regexp.MustCompile(`(?m:^- \[([ xX])\] (.+?)(?: \(checked by @([A-Za-z0-9-]+)\))?$)`)
 	// Regex for capturing only the checkboxes.
-	checkboxes = regexp.MustCompile(`(?m:^- \[[ x]\])`)
+	checkboxes = regexp.MustCompile(`(?m:^- \[[ xX]\])`)
 	// Regex used to capture markdown links.
 	markdownLink = regexp.MustCompile(`\[(.*)\]\([^)]*\)`)
 )
@@ -46,9 +46,11 @@ type ManualContent struct {
 	Teams            []string
 }
 type CommentContent struct {
-	AutoRules    []AutoContent
-	ManualRules  []ManualContent
-	allSatisfied bool
+	AutoRules          []AutoContent
+	ManualRules        []ManualContent
+	AutoAllSatisfied   bool
+	ManualAllSatisfied bool
+	ForceSkip          bool
 }
 
 type manualCheckDetails struct {
@@ -64,10 +66,10 @@ func getCommentManualChecks(commentBody string) map[string]manualCheckDetails {
 	// For each line that matches the "Manual check" regex.
 	for _, match := range manualCheckLine.FindAllStringSubmatch(commentBody, -1) {
 		description := match[2]
-		status := match[1]
+		status := strings.ToLower(match[1]) // if X captured, convert it to x.
 		checkedBy := ""
-		if len(match) > 4 {
-			checkedBy = strings.ToLower(match[4]) // if X captured, convert it to x.
+		if len(match) > 3 {
+			checkedBy = match[3]
 		}
 
 		checks[description] = manualCheckDetails{status: status, checkedBy: checkedBy}
@@ -261,13 +263,15 @@ func updatePullRequest(gh *client.GitHub, pr *github.PullRequest, content Commen
 	var (
 		context     = "Merge Requirements"
 		targetURL   = comment.GetHTMLURL()
-		state       = "failure"
-		description = "Some requirements are not satisfied yet. See bot comment."
+		state       = "success"
+		description = "All requirements are satisfied."
 	)
 
-	if content.allSatisfied {
-		state = "success"
-		description = "All requirements are satisfied."
+	if content.ForceSkip {
+		description = "Bot checks are skipped for this PR."
+	} else if !content.AutoAllSatisfied || !content.ManualAllSatisfied {
+		state = "failure"
+		description = "Some requirements are not satisfied yet. See bot comment."
 	}
 
 	// Update or create commit status.

contribs/github-bot/internal/check/comment.tmpl

@@ -1,19 +1,34 @@
-I'm a bot that assists the Gno Core team in maintaining this repository. My role is to ensure that contributors understand and follow our guidelines, helping to streamline the development process.
+#### 🛠 PR Checks Summary
+{{ if and .AutoRules (not .AutoAllSatisfied) }}{{ range .AutoRules }}{{ if not .Satisfied }} 🔴 {{ .Description }}
+{{end}}{{end}}{{ else }}All **Automated Checks** passed. ✅{{end}}
 
-The following requirements must be fulfilled before a pull request can be merged.
-Some requirement checks are automated and can be verified by the CI, while others need manual verification by a staff member.
+##### Manual Checks (for Reviewers):
+{{ if .ManualRules }}{{ range .ManualRules }}- [{{ if .CheckedBy }}x{{ else }} {{ end }}] {{ .Description }}{{ if .CheckedBy }} (checked by @{{ .CheckedBy }}){{ end }}
+{{ end }}{{ else }}*No manual checks match this pull request.*{{ end }}
 
-These requirements are defined in this [configuration file](https://github.com/gnolang/gno/tree/master/contribs/github-bot/internal/config/config.go).
+<details><summary>Read More</summary>
 
-## Automated Checks
+🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.
 
+##### ✅ Automated Checks (for Contributors):
 {{ if .AutoRules }}{{ range .AutoRules }} {{ if .Satisfied }}🟢{{ else }}🔴{{ end }} {{ .Description }}
 {{ end }}{{ else }}*No automated checks match this pull request.*{{ end }}
 
-## Manual Checks
+##### ☑️ Contributor Actions:
+1. Fix any issues flagged by automated checks.
+2. Follow the Contributor Checklist to ensure your PR is ready for review.
+	- Add new tests, or document why they are unnecessary.
+	- Provide clear examples/screenshots, if necessary.
+	- Update documentation, if required.
+	- Ensure no breaking changes, or include `BREAKING CHANGE` notes.
+	- Link related issues/PRs, where applicable.
 
-{{ if .ManualRules }}{{ range .ManualRules }}- [{{ if .CheckedBy }}x{{ else }} {{ end }}] {{ .Description }}{{ if .CheckedBy }} (checked by @{{ .CheckedBy }}){{ end }}
-{{ end }}{{ else }}*No manual checks match this pull request.*{{ end }}
+##### ☑️ Reviewer Actions:
+1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
+
+##### 📚 Resources:
+- [Report a bug with the bot](https://github.com/gnolang/gno/issues/3238).
+- [View the bot’s configuration file](https://github.com/gnolang/gno/tree/master/contribs/github-bot/internal/config/config.go).
 
 {{ if or .AutoRules .ManualRules }}<details><summary><b>Debug</b></summary><blockquote>
 {{ if .AutoRules }}<details><summary><b>Automated Checks</b></summary><blockquote>
@@ -52,3 +67,4 @@ These requirements are defined in this [configuration file](https://github.com/g
 {{ end }}
 </blockquote></details>
 {{ end }}
+</details>

contribs/github-bot/internal/check/comment_test.go

@@ -31,31 +31,44 @@ func TestGeneratedComment(t *testing.T) {
 		{Description: "Test automatic 5", Satisfied: false},
 	}
 	manualRules := []ManualContent{
-		{Description: "Test manual 1", CheckedBy: "user_1"},
+		{Description: "Test manual 1", CheckedBy: "user-1"},
 		{Description: "Test manual 2", CheckedBy: ""},
 		{Description: "Test manual 3", CheckedBy: ""},
-		{Description: "Test manual 4", CheckedBy: "user_4"},
-		{Description: "Test manual 5", CheckedBy: "user_5"},
+		{Description: "Test manual 4", CheckedBy: "user-4"},
+		{Description: "Test manual 5", CheckedBy: "user-5"},
 	}
 
 	commentText, err := generateComment(content)
 	assert.Nil(t, err, fmt.Sprintf("error is not nil: %v", err))
 	assert.True(t, strings.Contains(commentText, "*No automated checks match this pull request.*"), "should contains automated check placeholder")
 	assert.True(t, strings.Contains(commentText, "*No manual checks match this pull request.*"), "should contains manual check placeholder")
+	assert.True(t, strings.Contains(commentText, "All **Automated Checks** passed. ✅"), "should contains automated checks passed placeholder")
 
 	content.AutoRules = autoRules
+	content.AutoAllSatisfied = true
 	commentText, err = generateComment(content)
 	assert.Nil(t, err, fmt.Sprintf("error is not nil: %v", err))
 	assert.False(t, strings.Contains(commentText, "*No automated checks match this pull request.*"), "should not contains automated check placeholder")
 	assert.True(t, strings.Contains(commentText, "*No manual checks match this pull request.*"), "should contains manual check placeholder")
+	assert.True(t, strings.Contains(commentText, "All **Automated Checks** passed. ✅"), "should contains automated checks passed placeholder")
 	assert.Equal(t, 2, len(autoCheckSuccessLine.FindAllStringSubmatch(commentText, -1)), "wrong number of succeeded automatic check")
 	assert.Equal(t, 3, len(autoCheckFailLine.FindAllStringSubmatch(commentText, -1)), "wrong number of failed automatic check")
 
+	content.AutoAllSatisfied = false
+	commentText, err = generateComment(content)
+	assert.Nil(t, err, fmt.Sprintf("error is not nil: %v", err))
+	assert.False(t, strings.Contains(commentText, "*No automated checks match this pull request.*"), "should not contains automated check placeholder")
+	assert.True(t, strings.Contains(commentText, "*No manual checks match this pull request.*"), "should contains manual check placeholder")
+	assert.False(t, strings.Contains(commentText, "All **Automated Checks** passed. ✅"), "should contains automated checks passed placeholder")
+	assert.Equal(t, 2, len(autoCheckSuccessLine.FindAllStringSubmatch(commentText, -1)), "wrong number of succeeded automatic check")
+	assert.Equal(t, 3+3, len(autoCheckFailLine.FindAllStringSubmatch(commentText, -1)), "wrong number of failed automatic check")
+
 	content.ManualRules = manualRules
 	commentText, err = generateComment(content)
 	assert.Nil(t, err, fmt.Sprintf("error is not nil: %v", err))
 	assert.False(t, strings.Contains(commentText, "*No automated checks match this pull request.*"), "should not contains automated check placeholder")
 	assert.False(t, strings.Contains(commentText, "*No manual checks match this pull request.*"), "should not contains manual check placeholder")
+	assert.False(t, strings.Contains(commentText, "All **Automated Checks** passed. ✅"), "should contains automated checks passed placeholder")
 
 	manualChecks := getCommentManualChecks(commentText)
 	assert.Equal(t, len(manualChecks), len(manualRules), "wrong number of manual checks found")

contribs/github-bot/internal/conditions/fork.go

@@ -0,0 +1,27 @@
+package conditions
+
+import (
+	"fmt"
+
+	"github.com/gnolang/gno/contribs/github-bot/internal/utils"
+
+	"github.com/google/go-github/v64/github"
+	"github.com/xlab/treeprint"
+)
+
+// CreatedFromFork Condition.
+type createdFromFork struct{}
+
+var _ Condition = &createdFromFork{}
+
+func (b *createdFromFork) IsMet(pr *github.PullRequest, details treeprint.Tree) bool {
+	return utils.AddStatusNode(
+		pr.GetHead().GetRepo().GetFullName() != pr.GetBase().GetRepo().GetFullName(),
+		fmt.Sprintf("The pull request was created from a fork (head branch repo: %s)", pr.GetHead().GetRepo().GetFullName()),
+		details,
+	)
+}
+
+func CreatedFromFork() Condition {
+	return &createdFromFork{}
+}

contribs/github-bot/internal/conditions/fork_test.go

@@ -0,0 +1,31 @@
+package conditions
+
+import (
+	"testing"
+
+	"github.com/gnolang/gno/contribs/github-bot/internal/utils"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/google/go-github/v64/github"
+	"github.com/xlab/treeprint"
+)
+
+func TestCreatedFromFork(t *testing.T) {
+	t.Parallel()
+
+	var (
+		repo = &github.PullRequestBranch{Repo: &github.Repository{Owner: &github.User{Login: github.String("main")}, Name: github.String("repo"), FullName: github.String("main/repo")}}
+		fork = &github.PullRequestBranch{Repo: &github.Repository{Owner: &github.User{Login: github.String("fork")}, Name: github.String("repo"), FullName: github.String("fork/repo")}}
+	)
+
+	prFromMain := &github.PullRequest{Base: repo, Head: repo}
+	prFromFork := &github.PullRequest{Base: repo, Head: fork}
+
+	details := treeprint.New()
+	assert.False(t, CreatedFromFork().IsMet(prFromMain, details))
+	assert.True(t, utils.TestLastNodeStatus(t, false, details), "condition details should have a status: false")
+
+	details = treeprint.New()
+	assert.True(t, CreatedFromFork().IsMet(prFromFork, details))
+	assert.True(t, utils.TestLastNodeStatus(t, true, details), "condition details should have a status: true")
+}

contribs/github-bot/internal/config/config.go

@@ -22,13 +22,17 @@ type ManualCheck struct {
 	Teams       Teams       // Members of these teams can check the checkbox to make the check pass.
 }
 
+// This is the description for a persistent rule with a non-standard behavior
+// that allow maintainer to force the "success" state of the CI check
+const ForceSkipDescription = "**SKIP**: Do not block the CI for this PR"
+
 // This function returns the configuration of the bot consisting of automatic and manual checks
 // in which the GitHub client is injected.
 func Config(gh *client.GitHub) ([]AutomaticCheck, []ManualCheck) {
 	auto := []AutomaticCheck{
 		{
 			Description: "Maintainers must be able to edit this pull request ([more info](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork))",
-			If:          c.Always(),
+			If:          c.CreatedFromFork(),
 			Then:        r.MaintainerCanModify(),
 		},
 		{
@@ -53,6 +57,11 @@ func Config(gh *client.GitHub) ([]AutomaticCheck, []ManualCheck) {
 	}
 
 	manual := []ManualCheck{
+		{
+			// WARN: Do not edit this special rule which must remain persistent.
+			Description: ForceSkipDescription,
+			If:          c.Always(),
+		},
 		{
 			Description: "The pull request description provides enough details",
 			If:          c.Not(c.AuthorInTeam(gh, "core-contributors")),

contribs/github-bot/internal/matrix/matrix.go

@@ -113,7 +113,7 @@ func getPRListFromEvent(gh *client.GitHub, actionCtx *githubactions.GitHubContex
 
 	// Event triggered by an issue / PR comment being created / edited / deleted
 	// or any update on a PR.
-	case utils.EventIssueComment, utils.EventPullRequest, utils.EventPullRequestTarget:
+	case utils.EventIssueComment, utils.EventPullRequest, utils.EventPullRequestReview, utils.EventPullRequestTarget:
 		// For these events, retrieve the number of the associated PR from the context.
 		prNum, err := utils.GetPRNumFromActionsCtx(actionCtx)
 		if err != nil {

contribs/github-bot/internal/matrix/matrix_test.go

@@ -54,6 +54,15 @@ func TestProcessEvent(t *testing.T) {
 			prs,
 			utils.PRList{1},
 			false,
+		}, {
+			"valid pull_request_review event",
+			&githubactions.GitHubContext{
+				EventName: utils.EventPullRequestReview,
+				Event:     map[string]any{"pull_request": map[string]any{"number": 1.}},
+			},
+			prs,
+			utils.PRList{1},
+			false,
 		}, {
 			"valid pull_request_target event",
 			&githubactions.GitHubContext{

contribs/github-bot/internal/utils/actions.go

@@ -30,7 +30,7 @@ func GetPRNumFromActionsCtx(actionCtx *githubactions.GitHubContext) (int, error)
 	switch actionCtx.EventName {
 	case EventIssueComment:
 		firstKey = "issue"
-	case EventPullRequest, EventPullRequestTarget:
+	case EventPullRequest, EventPullRequestReview, EventPullRequestTarget:
 		firstKey = "pull_request"
 	default:
 		return 0, fmt.Errorf("unsupported event: %s", actionCtx.EventName)

contribs/github-bot/internal/utils/github_const.go

@@ -5,6 +5,7 @@ const (
 	// GitHub Actions Event Names.
 	EventIssueComment      = "issue_comment"
 	EventPullRequest       = "pull_request"
+	EventPullRequestReview = "pull_request_review"
 	EventPullRequestTarget = "pull_request_target"
 	EventWorkflowDispatch  = "workflow_dispatch"