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

Export CompatFilePath and CompatLicenseFile #8523

Conversation

andreabedini
Copy link
Collaborator

These two types need to be exported for users to be able to define their own FieldGrammar.


Please include the following checklist in your PR:

Please also shortly describe how you tested your change. Bonus points for added tests!

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

Copy link
Member

@Kleidukos Kleidukos left a comment

Choose a reason for hiding this comment

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

LGTM

@andreabedini andreabedini removed the request for review from Mikolaj October 13, 2022 01:59
@andreabedini andreabedini force-pushed the andrea/packagedescription-fieldgrammar-exports branch from b756c6d to 562b6f8 Compare October 13, 2022 02:00
These two types need to be exported for users to be able to
define their own FieldGrammar.
@andreabedini andreabedini force-pushed the andrea/packagedescription-fieldgrammar-exports branch from 562b6f8 to 8edc006 Compare October 14, 2022 15:06
@andreabedini andreabedini merged commit bb187b8 into haskell:master Oct 15, 2022
@andreabedini andreabedini deleted the andrea/packagedescription-fieldgrammar-exports branch October 15, 2022 08:56
@ulysses4ever
Copy link
Collaborator

Just curious: any reason not to use the merge bot?

@andreabedini
Copy link
Collaborator Author

@ulysses4ever oh damn it, I forgot. Is there a way to disable the green button? Or perhaps take away my merge permission, I’m just an occasional contributor after all.

@ulysses4ever
Copy link
Collaborator

@andreabedini right: it's super easy to forget, I agree. As to how to improve on this, I'm not totally sure, but some directions that can be investigated:

The former seems to be the proper solution. Unfortunately, it requires GitHub Team or GitHub Enterprise Cloud organization account. I'm guessing, haskell doesn't have it?

What are downsides of the latter? I think, the only sensitive change is that people won't be able to push their dev branches to haskell/cabal and will have to use their forks of the cabal repo. I have to admit, this may be a bit annoying (it's just more expedite and straightforward to be able to work with one remote instead of two). But this is certainly not the end of the world.

@Mikolaj do you have any thoughts on this?

Meanwhile, we just had another direct merge. I'd guess the reason is the same: the person wasn't aware about the bot but had Write permissions.

@andreabedini
Copy link
Collaborator Author

andreabedini commented Oct 19, 2022

I cannot speak for other developers but I would be ok with Triage level.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 19, 2022

@andreabedini: you've been awarded the coveted Triage role. Thank you for your exemplary Write service.

We are advised by Haskell org admins to prefer the Triage role, so slowly migrating that way makes sense. I don't see in Settings the possibility of restricting the Merge button, so I guess we can't.

@ulysses4ever
Copy link
Collaborator

@Mikolaj

We are advised by Haskell org admins to prefer the Triage role, so slowly migrating that way makes sense.

any reason in the "slowly" bit in particular? Wouldn't it be better to do a bulk rollback of Write permissions and be done with it? The interface at github.com/<owner>/<repo>/settings/access seems pretty handy for that sort of operations: you can filter by the write role, "Select All" and change the role to triage.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 20, 2022

I'd be miffed if anybody took away my Write permission. ;)

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.

None yet

4 participants