Set file encoding in GitLab commits #7381
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jurre
reviewed
Jun 5, 2023
There was a problem hiding this comment.
Overall changes look good, I made a few suggestions and had one question, lmk your thoughts.
Internally we have a few stability improvements that we need to focus on first before we can accept any contributions, so it'll take a bit of time before we can merge this, just FYI.
| @@ -124,6 +125,15 @@ def file_action(file) | |||
| end | |||
| end | |||
|
|
|||
| # @param [DependencyFile] file | |||
| def file_encoding(file) | |||
| if file.content_encoding == Dependabot::DependencyFile::ContentEncoding::BASE64 | |||
There was a problem hiding this comment.
Suggested change
| if file.content_encoding == Dependabot::DependencyFile::ContentEncoding::BASE64 | |
| if file.binary? |
| @@ -108,7 +108,8 @@ def file_actions | |||
| { | |||
| action: file_action(file), | |||
| file_path: file.type == "symlink" ? file.symlink_target : file.path, | |||
| content: file.content | |||
| content: file.content, | |||
| **file_encoding(file) | |||
There was a problem hiding this comment.
Could we pass file.content_encoding directly? It would be either utf-8 or base64
There was a problem hiding this comment.
I did it like this for two reasons:
- I didn't want to change the behavior for the other case
- I don't want to make any assumption that the string values of constants defined for another purpose match the input required by the GitLab API, even if they happen to do so.
But I'll change it to pass file.content_encoding directly if you think that is better
mikaellanger
force-pushed
the
langer-patch-gitlab-content-type
branch
from
0d2b0db
to
04a45f0
Compare
jurre
force-pushed
the
langer-patch-gitlab-content-type
branch
from
04a45f0
to
6804ba9
Compare
jurre
force-pushed
the
langer-patch-gitlab-content-type
branch
from
6804ba9
to
0deedaf
Compare
jeffwidman
previously approved these changes
Jul 24, 2023
jeffwidman
dismissed
their stale review
Overlooked that Jurre's suggestions on binary weren't fully implemented... please do that first.
@jeffwidman Not sure I understand what you mean? That code was removed in favor of setting encoding directly to file.content_encoding per the other comment by Jurre #7381 (comment) |
jeffwidman
reviewed
Aug 3, 2023
jeffwidman
force-pushed
the
langer-patch-gitlab-content-type
branch
from
a0173bf
to
e158497
Compare
|
@mikaellanger @jeffwidman GitLab API docs says that encoding should be either text or base64, not utf-8. Text is default. Setting |
brettfo
pushed a commit
to brettfo/dependabot-core
that referenced
this pull request
Oct 11, 2023
Previously `request_{creator,updater}/gitlab.rb` didn't pass the DependencyFile `content_encoding` to the GitLab API. This caused vendored files (in our case `yarn` `pnp` zip files) to be uploaded as text.
---------
Co-authored-by: Mikael Langer <mikael.langer@safeture.com>
Co-authored-by: Jeff Widman <jeff@jeffwidman.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
request_{creator,updater}/gitlab.rb doesn't pass the DependencyFile content_encoding to the GitLab API. This causes vendored files (in our case yarn pnp zip files) to be uploaded as text.