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.

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

[Reverted] GitHub Actions to add/remove label "stale" and comment #7950

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

totoprayogo1916
Copy link
Contributor

@totoprayogo1916 totoprayogo1916 commented Sep 17, 2023

Description
GA concept:

  • Any changes made in the main branch will be automatically reviewed against all open pull requests (PRs). If a conflict arises, a label "stale" will be added, and a comment will be left to notify the contributors.
  • Upon an update to a PR, a reevaluation will be conducted. If the PR has the "stale" label and there are no conflicts, the label will be removed.

Live Test: https://github.com/totoprayogo1916/github-playground/pull/13

Checklist:

  • Securely signed commits

@totoprayogo1916 totoprayogo1916 deleted the confl branch September 17, 2023 14:45
@totoprayogo1916 totoprayogo1916 restored the confl branch September 18, 2023 17:15
@totoprayogo1916 totoprayogo1916 changed the title Confl GitHub Actions to add/remove label "stale" and comment Sep 19, 2023
@totoprayogo1916
Copy link
Contributor Author

cc @kenjis @ddevsr @paulbalandan

@ddevsr ddevsr added the github_actions Pull requests that update Github_actions code label Sep 19, 2023
Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

Oh, thank you! This seems good.

@kenjis kenjis merged commit 3f5ac0d into codeigniter4:develop Sep 20, 2023
3 checks passed
@totoprayogo1916 totoprayogo1916 deleted the confl branch September 20, 2023 05:03
@kenjis
Copy link
Member

kenjis commented Sep 20, 2023

This workflow labeled for PRs with no conflicts, so I reverted.
#7947 (comment)
#7944 (comment)
#7925 (comment)

labels=$(echo "$pr" | jq -c '.labels[].name' | tr -d '[]"')
url=$(echo "$pr" | jq -r '.url')

if [ "$mergeable" != "MERGEABLE" ] && [[ ! "$labels" == *"stale"* ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kenjis I believe the outcome of $mergeable yields an "UNKNOWN" result. Upon investigation, I discovered that $mergeable can result in three potential outcomes: "MERGEABLE," "CONFLICTING," or "UNKNOWN."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we could organize it in a way where "CONFLICTING" results take precedence.

Copy link
Member

Choose a reason for hiding this comment

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

But if it is "UNKNOWN", we cannot put a label.

At least, I can see the outcomes:

$ gh pr list -L 100 --json mergeable,url 
[
  {
    "mergeable": "CONFLICTING",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7951"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7947"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7946"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7944"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7933"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7925"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7924"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7846"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7732"
  },
  {
    "mergeable": "CONFLICTING",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7724"
  },
  {
    "mergeable": "CONFLICTING",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7718"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7717"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7491"
  },
  {
    "mergeable": "CONFLICTING",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/7404"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/6877"
  },
  {
    "mergeable": "CONFLICTING",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/6845"
  },
  {
    "mergeable": "CONFLICTING",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/6284"
  },
  {
    "mergeable": "MERGEABLE",
    "url": "https://github.com/codeigniter4/CodeIgniter4/pull/1797"
  }
]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it can be changed like this?

if [ "$mergeable" == "CONFLICTING" ] &&[[ ! "$labels" == *"stale"* ]]; then
    ...

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is safe.

Copy link
Contributor Author

@totoprayogo1916 totoprayogo1916 Sep 20, 2023

Choose a reason for hiding this comment

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

The team point out that the merge state needs to be recalculated when the base branch changes. Since the codeignitier PR is based on their develop branch (https://github.com/codeigniter4/CodeIgniter4/tree/develop), it would make sense that there are periods of time where a PR mergeable field is UNKNOWN.

I hope this answers your question 🙏

got the answer, why UNKNOWN on $mergeable
cli/cli#8020 (reply in thread)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Can you send another PR with "$mergeable" == "CONFLICTING"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

created #7957

@kenjis kenjis changed the title GitHub Actions to add/remove label "stale" and comment [Reverted] GitHub Actions to add/remove label "stale" and comment Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update Github_actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants