You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/06/07 08:31:00 UTC

[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #2096: Add TagsPattern for use Pattern in refdiff

warren830 commented on code in PR #2096:
URL: https://github.com/apache/incubator-devlake/pull/2096#discussion_r890928444


##########
plugins/refdiff/refdiff.go:
##########
@@ -83,19 +85,34 @@ func main() {
 	newRef := refdiffCmd.Flags().StringP("new-ref", "n", "", "new ref")
 	oldRef := refdiffCmd.Flags().StringP("old-ref", "o", "", "old ref")
 
+	tagsPattern := refdiffCmd.Flags().StringP("tags-pattern", "p", "", "tags pattern")
+	tagsLimit := refdiffCmd.Flags().StringP("tags-limit", "l", "", "tags limit")
+	tagsOrder := refdiffCmd.Flags().StringP("tags-order", "d", "", "tags order")

Review Comment:
   can you write some comments for these three variables?



##########
plugins/refdiff/refdiff.go:
##########
@@ -83,19 +85,34 @@ func main() {
 	newRef := refdiffCmd.Flags().StringP("new-ref", "n", "", "new ref")
 	oldRef := refdiffCmd.Flags().StringP("old-ref", "o", "", "old ref")
 
+	tagsPattern := refdiffCmd.Flags().StringP("tags-pattern", "p", "", "tags pattern")
+	tagsLimit := refdiffCmd.Flags().StringP("tags-limit", "l", "", "tags limit")
+	tagsOrder := refdiffCmd.Flags().StringP("tags-order", "d", "", "tags order")
+
 	_ = refdiffCmd.MarkFlagRequired("repo-id")
-	_ = refdiffCmd.MarkFlagRequired("new-ref")
-	_ = refdiffCmd.MarkFlagRequired("old-ref")
+	//_ = refdiffCmd.MarkFlagRequired("new-ref")
+	//_ = refdiffCmd.MarkFlagRequired("old-ref")
 
 	refdiffCmd.Run = func(cmd *cobra.Command, args []string) {
+		tl, _ := strconv.Atoi(*tagsLimit)

Review Comment:
   why swallow the err?



##########
plugins/refdiff/tasks/refdiff_task_data.go:
##########
@@ -36,3 +46,77 @@ type RefPair struct {
 	NewRef string
 	OldRef string
 }
+
+type Refs []code.Ref
+type RefsAlphabetically Refs
+type RefsReverseAlphabetically Refs
+
+func (rs Refs) Len() int {
+	return len(rs)
+}
+
+func (rs RefsAlphabetically) Len() int {
+	return len(rs)
+}
+
+func (rs RefsAlphabetically) Less(i, j int) bool {
+	return rs[i].Id < rs[j].Id
+}
+
+func (rs RefsAlphabetically) Swap(i, j int) {
+	rs[i], rs[j] = rs[j], rs[i]
+}
+
+func (rs RefsReverseAlphabetically) Len() int {
+	return len(rs)
+}
+
+func (rs RefsReverseAlphabetically) Less(i, j int) bool {
+	return rs[i].Id > rs[j].Id
+}
+
+func (rs RefsReverseAlphabetically) Swap(i, j int) {
+	rs[i], rs[j] = rs[j], rs[i]
+}
+
+func CaculateTagPattern(taskCtx core.SubTaskContext) (Refs, error) {
+	rs := Refs{}
+	data := taskCtx.GetData().(*RefdiffTaskData)
+	tagsPattern := data.Options.TagsPattern
+	tagsOrder := data.Options.TagsOrder
+	db := taskCtx.GetDb()
+
+	// caculate Pattern part
+	if data.Options.TagsPattern == "" || data.Options.TagsLimit <= 1 {
+		return rs, nil
+	}
+	rows, err := db.Model(&code.Ref{}).Order("created_date desc").Rows()
+	if err != nil {
+		return rs, err
+	}
+	defer rows.Next()
+	r, err := regexp.Compile(tagsPattern)
+	if err != nil {
+		return rs, fmt.Errorf("unable to parse: %s\r\n%s", tagsPattern, err.Error())
+	}
+	for rows.Next() {
+		var ref code.Ref
+		err = db.ScanRows(rows, &ref)
+		if err != nil {
+			return rs, err
+		}
+
+		if ok := r.Match([]byte(ref.Id)); ok {
+			rs = append(rs, ref)
+		}
+	}
+	switch tagsOrder {
+	case "alphabetically":

Review Comment:
   I think `alphabetically` is too long for an arg



##########
plugins/refdiff/tasks/ref_commit_diff_calculator.go:
##########
@@ -27,14 +27,27 @@ import (
 	"gorm.io/gorm/clause"
 )
 
-func CalculateCommitsDiff(taskCtx core.SubTaskContext) error {
+func CalculateCommitsPairs(taskCtx core.SubTaskContext) ([][4]string, error) {
 	data := taskCtx.GetData().(*RefdiffTaskData)
 	repoId := data.Options.RepoId
 	pairs := data.Options.Pairs
+	tagsLimit := data.Options.TagsLimit
 	db := taskCtx.GetDb()
-	ctx := taskCtx.GetContext()
-	logger := taskCtx.GetLogger()
-	insertCountLimitOfRefsCommitsDiff := int(65535 / reflect.ValueOf(code.RefsCommitsDiff{}).NumField())
+
+	rs, err := CaculateTagPattern(taskCtx)

Review Comment:
   same as above



##########
plugins/refdiff/tasks/ref_issue_diff_calculator.go:
##########
@@ -26,15 +26,40 @@ import (
 	"github.com/apache/incubator-devlake/plugins/helper"
 )
 
-func CalculateIssuesDiff(taskCtx core.SubTaskContext) error {
+func CaculatePairList(taskCtx core.SubTaskContext) ([][2]string, error) {

Review Comment:
   please write comment



##########
plugins/refdiff/refdiff.go:
##########
@@ -83,19 +85,34 @@ func main() {
 	newRef := refdiffCmd.Flags().StringP("new-ref", "n", "", "new ref")
 	oldRef := refdiffCmd.Flags().StringP("old-ref", "o", "", "old ref")
 
+	tagsPattern := refdiffCmd.Flags().StringP("tags-pattern", "p", "", "tags pattern")
+	tagsLimit := refdiffCmd.Flags().StringP("tags-limit", "l", "", "tags limit")
+	tagsOrder := refdiffCmd.Flags().StringP("tags-order", "d", "", "tags order")
+
 	_ = refdiffCmd.MarkFlagRequired("repo-id")

Review Comment:
   if we don't process the returned err, why do we call `MarkFlagRequired`
   



##########
plugins/refdiff/tasks/ref_commit_diff_calculator.go:
##########
@@ -27,14 +27,27 @@ import (
 	"gorm.io/gorm/clause"
 )
 
-func CalculateCommitsDiff(taskCtx core.SubTaskContext) error {
+func CalculateCommitsPairs(taskCtx core.SubTaskContext) ([][4]string, error) {

Review Comment:
   and what is the return?



##########
plugins/refdiff/tasks/ref_commit_diff_calculator.go:
##########
@@ -27,14 +27,27 @@ import (
 	"gorm.io/gorm/clause"
 )
 
-func CalculateCommitsDiff(taskCtx core.SubTaskContext) error {
+func CalculateCommitsPairs(taskCtx core.SubTaskContext) ([][4]string, error) {

Review Comment:
   please write comment to describe what does this func do



##########
plugins/refdiff/tasks/refdiff_task_data.go:
##########
@@ -36,3 +46,77 @@ type RefPair struct {
 	NewRef string
 	OldRef string
 }
+
+type Refs []code.Ref
+type RefsAlphabetically Refs
+type RefsReverseAlphabetically Refs
+
+func (rs Refs) Len() int {
+	return len(rs)
+}
+
+func (rs RefsAlphabetically) Len() int {
+	return len(rs)
+}
+
+func (rs RefsAlphabetically) Less(i, j int) bool {
+	return rs[i].Id < rs[j].Id
+}
+
+func (rs RefsAlphabetically) Swap(i, j int) {
+	rs[i], rs[j] = rs[j], rs[i]
+}
+
+func (rs RefsReverseAlphabetically) Len() int {
+	return len(rs)
+}
+
+func (rs RefsReverseAlphabetically) Less(i, j int) bool {
+	return rs[i].Id > rs[j].Id
+}
+
+func (rs RefsReverseAlphabetically) Swap(i, j int) {
+	rs[i], rs[j] = rs[j], rs[i]
+}
+
+func CaculateTagPattern(taskCtx core.SubTaskContext) (Refs, error) {

Review Comment:
   please write comment



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org