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

Jump to bottom

Remove functionality to add, view and edit binary files in gists #3042

Merged
merged 4 commits into from Mar 4, 2021
Merged

Remove functionality to add, view and edit binary files in gists #3042

merged 4 commits into from Mar 4, 2021
+110 −10

Conversation

g14a
Copy link
Contributor

@g14a g14a commented Feb 26, 2021

This commit adds file validation to check if file contents are binary in nature, and if so halts creation, edition and rendering of contents directly to stdout.

Closes #2998

@g14a g14a mentioned this pull request Feb 26, 2021
Closed
mislav
mislav requested changes Feb 26, 2021
View changes
Copy link
Member

@mislav mislav left a comment

Thanks! This is going in a good direction. A few requests:

pkg/cmd/gist/view/view.go Outdated Show resolved Hide resolved
pkg/cmd/gist/create/create.go Outdated Show resolved Hide resolved
pkg/cmd/gist/view/view.go Outdated Show resolved Hide resolved
pkg/cmd/gist/view/view.go Outdated Show resolved Hide resolved
pkg/cmd/gist/view/view.go Outdated Show resolved Hide resolved
@g14a
Copy link
Contributor Author

g14a commented Feb 26, 2021

@mislav I've updated the PR and standardized some options. Please check at your leisure. Hope the changes I've made are for the better and not the worse.

@g14a
Copy link
Contributor Author

g14a commented Mar 1, 2021

Anything else to be done here? @mislav

mislav
mislav requested changes Mar 1, 2021
View changes
Copy link
Member

@mislav mislav left a comment

Almost there! Thank you for your patience

pkg/cmd/gist/edit/edit.go Outdated Show resolved Hide resolved
pkg/cmd/gist/view/view.go Outdated Show resolved Hide resolved
pkg/cmd/gist/shared/shared.go Outdated Show resolved Hide resolved
pkg/cmd/gist/view/view.go Outdated Show resolved Hide resolved
@g14a
Copy link
Contributor Author

g14a commented Mar 2, 2021

Thank you for the feedback @mislav it helped a lot. I've updated the PR.

@mislav mislav force-pushed the bug/gist-binary-files branch from e280832 to 066ba54 Compare Mar 2, 2021
mislav
mislav approved these changes Mar 2, 2021
View changes
Copy link
Member

@mislav mislav left a comment

Thank you! It looks great. I've pushed some tweaks to simplify and to handle the new gist edit -a flag

@mislav mislav requested a review from a team Mar 2, 2021
@mislav mislav added this to Backlog 🗒 in The GitHub CLI via automation Mar 2, 2021
@mislav mislav moved this from Backlog 🗒 to Needs review 🤔 in The GitHub CLI Mar 2, 2021
vilmibm
vilmibm approved these changes Mar 4, 2021
View changes
The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Mar 4, 2021
@vilmibm vilmibm merged commit 2fbc037 into cli:trunk Mar 4, 2021
7 checks passed
The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Mar 4, 2021
@github-actions github-actions bot moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Mar 30, 2021
@hongoctoan

This comment was marked as spam.

Sign in to view
@hongoctoan

This comment was marked as spam.

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

@mislav mislav

@vilmibm vilmibm

Assignees
No one assigned
Labels
None yet
Projects
The GitHub CLI
  
Done 💤
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gist creation mangles binary files
4 participants
@g14a @hongoctoan @mislav @vilmibm