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

labels: prevent new labels being created #80

Merged
merged 3 commits into from
Oct 18, 2016

Conversation

phillipj
Copy link
Member

As @jasnell reported in #58, the bot (unexpectedly) creates new repo labels once in a while. These changes prevents that from happening, by checking which labels already exists in the repo, before adding labels to PRs.

Closes #58

These changes prevents the bot from creating new labels which doesn't
already exist in the node repo.

Closes nodejs#58
@phillipj phillipj force-pushed the prevent-creating-labels branch from 7209a38 to d4b3edc Compare September 22, 2016 13:33
@phillipj
Copy link
Member Author

@nodejs/github-bot anyone willing to approve and share the blame on this one?

@Fishrock123
Copy link
Contributor

Didn't we originally do this? I have a recollection of it.

@phillipj
Copy link
Member Author

Earlier we fetched which labels were already added to a PR before adding
additional resolved labels, assume that's what you're referring to.

This is not related, as this ensure new repo labels aren't created in the
node repo. As @jasnell discovered, the bot might end up creating completely
new labels that doesn't already exist, cause a PR has created a new file in
the lib-directory, e.g lib/mapproxy.js as the PR he mentioned.

On Tuesday, 27 September 2016, Jeremiah Senkpiel [email protected]
wrote:

Didn't we originally do this? I have a recollection of it.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#80 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABLLEwCSKHFgJFu36R2vgRXG_InCmTtUks5quSpagaJpZM4KA7Pg
.

@Fishrock123
Copy link
Contributor

Oh right. Do we need to do this for every check, or can we do it like.. once every hour?

@phillipj
Copy link
Member Author

Good idea, once an hour should be sufficient. I'll fix

On Tuesday, 27 September 2016, Jeremiah Senkpiel [email protected]
wrote:

Oh right. Do we need to do this for every check, or can we do it like..
once every hour?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#80 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABLLE4jPImtGV7N-TjgFNBznDX6s34ssks5quTprgaJpZM4KA7Pg
.

@phillipj
Copy link
Member Author

Long overdue, but I just pushed cache w/1 hour expiry for the existing repo labels as suggested.

PTAL

@phillipj
Copy link
Member Author

@nodejs/github-bot this has stalled a bit, but I'll merge this within a day or two if there's no more comments or objections

@phillipj phillipj merged commit 3f6d0e8 into nodejs:master Oct 18, 2016
@phillipj phillipj deleted the prevent-creating-labels branch October 18, 2016 18:12
phillipj added a commit to phillipj/github-bot that referenced this pull request Oct 19, 2016
After PR nodejs#80 got merged,
no labels were added to new PRs.

That regression probably came from the fact that the existing repository
labels was fetched and compared to the label names our labelling algorithm
thought we should add. The problem is that we compared the whole
label meta object (from api.github.com) against a label name (string).

Example of the two different types of objects:

```
// complete label object
{
  "url": "https://api.github.com/repos/nodejs/node/labels/buffer",
  "name": "buffer",
  "color": "f7c6c7"
}

// label name
"buffer"

```

Comparing two such objects will obviuosly never match.

These changes extracts the existing repo label *names* for comparison,
and adds more logging for future debugging purposes.
phillipj added a commit that referenced this pull request Oct 19, 2016
After PR #80 got merged,
no labels were added to new PRs.

That regression probably came from the fact that the existing repository
labels was fetched and compared to the label names our labelling algorithm
thought we should add. The problem is that we compared the whole
label meta object (from api.github.com) against a label name (string).

Example of the two different types of objects:

```
// complete label object
{
  "url": "https://api.github.com/repos/nodejs/node/labels/buffer",
  "name": "buffer",
  "color": "f7c6c7"
}

// label name
"buffer"

```

Comparing two such objects will obviuosly never match.

These changes extracts the existing repo label *names* for comparison,
and adds more logging for future debugging purposes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants