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

Allow option to pass file mode when opening a PR #32

Open
LucaLanziani opened this issue Jun 1, 2020 · 6 comments
Open

Allow option to pass file mode when opening a PR #32

LucaLanziani opened this issue Jun 1, 2020 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@LucaLanziani
Copy link
Contributor

Proposal to pass file mode when opening a PR:

Use case

PR containing a bash script that should have the executable flag set

Proposal

Allow the files values to be either a string, null or an object.

If the value is an object, the object must contain the key content that can either be a string or null.

The object might contain the mode key that will override the default 100644 mode for the specific file.

Implementation

I've already drafted a possible implementation in case you are willing to accept this change.

https://github.com/LucaLanziani/octokit-create-pull-request/pull/1/files

@gr2m
Copy link
Owner

gr2m commented Jun 1, 2020

have you seen #31? It implements the ability to create/update binary files by setting a file to an object with content and encoding keys.

I think setting a file to null is already implemented?

@LucaLanziani
Copy link
Contributor Author

LucaLanziani commented Jun 5, 2020

If I understand correctly that PR allows you to attach binary files, I'm just trying to set the executable flag to a bash script for instance. Do you want me to rebase my PR on top of #31 changes?

@gr2m
Copy link
Owner

gr2m commented Jun 5, 2020

Ah I see. Yes, if you could rebase your changes and start a PR, we can continue discussing there. Thanks Luca!

@gr2m gr2m added enhancement New feature or request help wanted Extra attention is needed labels Jun 16, 2020
@gr2m
Copy link
Owner

gr2m commented Jun 16, 2020

Hey @LucaLanziani, if you are anyone else is still interested, I'd be happy to review a PR. Note that I made quite a few changes in the past few days, it might be easiest to recreate the changes based on that.

I also added instructions to CONTRIBUTING.md on how to record fixtures for the tests.

@LucaLanziani
Copy link
Contributor Author

I'm doing my best to find some time to implement this, on a side note I also find a case where I would like to override the fork behavior here and return an error if the user doesn't have permission to create the PR.
Ideally the user would pass and option to the function to disable the fork, I would open a new issue if you are ok to add this option.

@gr2m
Copy link
Owner

gr2m commented Jun 18, 2020

I would open a new issue if you are ok to add this option

Yes, let's open an issue. I'd like to learn more about your use case and see if others have that similar use cases, too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants