Skip to content

fix #2626 in 2.0 #2678

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

Merged
merged 3 commits into from
Mar 24, 2023
Merged

fix #2626 in 2.0 #2678

merged 3 commits into from
Mar 24, 2023

Conversation

johrstrom
Copy link
Contributor

@johrstrom johrstrom commented Mar 20, 2023

Fix #2626 in 2.0. Can't directly use the commit from #2626, so it's a new commit.

┆Issue is synchronized with this Asana task by Unito

@johrstrom johrstrom marked this pull request as ready for review March 20, 2023 15:21
Comment on lines 25 to 27
valid_encoding = stats[:name].to_s.valid_encoding?
Rails.logger.warn("Not showing file '#{stats[:name]}' because it is not a UTF-8 filename.") unless valid_encoding
valid_encoding
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good, but for readability could we just do:

valid_encoding = stats[:name].to_s.valid_encoding?

unless valid_encoding
   Rails.logger.warn("Not showing file '#{stats[:name]}' because it is not a UTF-8 filename.")
end

valid_encoding

@Oglopf
Copy link
Contributor

Oglopf commented Mar 21, 2023

I'm not sure what is causing the tests to fail on this PR. They don't seem related to the change, but am I missing something?

@johrstrom
Copy link
Contributor Author

I'm not sure what is causing the tests to fail on this PR. They don't seem related to the change, but am I missing something?

I don't know, 2.0 tests just started failing at some point? #2632 fixes tests in 2.0.

@johrstrom johrstrom closed this Mar 24, 2023
@johrstrom johrstrom reopened this Mar 24, 2023
@johrstrom johrstrom merged commit 42f61f1 into release_2.0 Mar 24, 2023
@johrstrom johrstrom deleted the backport-non-utf-8 branch March 24, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants