Skip to content

Conversation

GeoffreyBooth
Copy link
Member

Closes #45

@GeoffreyBooth GeoffreyBooth added the documentation Improvements or additions to documentation label Oct 26, 2021
Copy link
Member

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

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

🙌

Sure, or we could delete that line.

Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
@JakobJingleheimer
Copy link
Member

O.o weird. why does my approval not fully register anymore? (it says "1 approval", and the checkmark next to my seemingly unofficial approval is greyed out instead of green)

@DerekNonGeneric
Copy link
Contributor

@JakobJingleheimer, perhaps because you are not a core collaborator, but if anyone knows how to give you commit access to this repo, it seems like that would be appropriate at this point. That privilege should probably also be extended to all of the other team members as well, but I don't know how to do it. It used to be possible, but I can't seem to find the settings necessary to grant others this power. (?)

If anyone can find out how (perhaps in the GitHub docs), I would gladly give you all the commit bit.

@JakobJingleheimer
Copy link
Member

I surely already have "Write" permissions, which is supposed to enable

Read, clone, and push. Can also manage issues and pull requests.

I would have guessed "manage issues and pull requests" would include approving, but perhaps not.

So maybe it takes granting "Maintain" permissions on this repo (to the @nodejs/loaders team)? I dunno what "manage […] some repository settings" means.

https://docs.github.com/en/organizations/managing-access-to-your-organizations-repositories/managing-team-access-to-an-organization-repository

P.S. I already have 2FA set up (I think membership in the nodejs org requires that anyway).

@GeoffreyBooth
Copy link
Member Author

I think it’s probably just because I committed your suggestion after you had approved the PR. So your approval was for an earlier version of this PR, not the current one. I bet if you approve again now, it’ll appear as green.

@JakobJingleheimer
Copy link
Member

Hmm, I just tried, but it won't let me because it says I already approved it, so I tried to dismiss my existing review, and it won't let me do that either:

image

I checked previous PRs in this repo I approved where there were no changes after, and they also display as unofficial.

@targos
Copy link
Member

targos commented Oct 27, 2021

This looks like a github bug. I would try to contact support if it isn't resolved today

@GeoffreyBooth GeoffreyBooth merged commit ce2c4e0 into main Oct 27, 2021
@GeoffreyBooth GeoffreyBooth deleted the 2021-10-26 branch October 27, 2021 17:18
@JakobJingleheimer
Copy link
Member

From GitHub support:

The the account JakobJingleheimer is a direct collaborator with Triage role in the organization. This role allows a user to manage issues and pull requests without write access. However, only reviews by reviewers with write access count toward mergeability.

So perhaps I/we could have the next-up level of permissions on this repo?

@GeoffreyBooth
Copy link
Member Author

GeoffreyBooth commented Nov 3, 2021

It looks like the loaders group had Triage role, so I changed the whole group to have the Write role. The GitHub UI doesn’t exactly make the difference clear when it comes to the mergeability of pull requests:

image

cc @MylesBorins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node.js Loaders Team Meeting 2021-10-26

4 participants