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

Eliminate CRLF from the copied flutter directory for Docker build #9298

Closed
wants to merge 18 commits into from

Conversation

AlexV525
Copy link
Member

@AlexV525 AlexV525 commented Aug 25, 2023

Force eliminate CRLF from the copied flutter directory.

Issues fixed (partially) by this PR (if any):

Presubmit checklist

@github-actions
Copy link

github-actions bot commented Aug 25, 2023

Visit the preview URL for this PR (updated for commit 0a78c2e):

https://flutter-docs-prod--pr9298-fix-crlf-to-lf-i9xidbu2.web.app

(expires Sat, 02 Sep 2023 18:46:19 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: d5ba327eec813901cac8396c4f458b02288624ab

@atsansone
Copy link
Contributor

atsansone commented Aug 25, 2023

@AlexV525 : I like this change. Did it work on your machine? (I ask because I have no easy way to test.)

@atsansone atsansone self-assigned this Aug 25, 2023
@atsansone atsansone added the review.tech Awaiting Technical Review label Aug 25, 2023
@AlexV525
Copy link
Member Author

I didn't run the full process but the flutter one works.
image

@parlough
Copy link
Member

I haven't had access to a Windows setup for a while, but I'll work on getting something set up this weekend.

Thanks for staying on top of this Alex! Would you mind adding a comment to explain the line? I think it might be easy to forget why it's there in the future.

@AlexV525
Copy link
Member Author

AlexV525 commented Aug 25, 2023

Would you mind adding a comment to explain the line? I think it might be easy to forget why it's there in the future.

Oh, I just place the command as an alias: REPLACE_CRLF, is it self-explaining enough?

@AlexV525
Copy link
Member Author

Nah, it not actually working since my clone is already in LF.

@AlexV525 AlexV525 marked this pull request as draft August 25, 2023 18:32
@AlexV525
Copy link
Member Author

AlexV525 commented Aug 25, 2023

Thanks for the ChatGPT, the command is now working.
image

@AlexV525 AlexV525 marked this pull request as ready for review August 25, 2023 19:01
@AlexV525 AlexV525 marked this pull request as draft August 25, 2023 19:55
@AlexV525 AlexV525 marked this pull request as ready for review August 26, 2023 06:47
@AlexV525
Copy link
Member Author

Notice that this only resolves a single part of the whole setup process.

@AlexV525
Copy link
Member Author

Alternatively, we can always clone the flutter repo rather than copying.

@AlexV525
Copy link
Member Author

Alternatively, we can always clone the flutter repo rather than copying.

I'd perfer this after some experiments. Will submit a separate PR for it. And you guys can decide which to take.

@AlexV525 AlexV525 marked this pull request as draft August 26, 2023 11:07
@AlexV525
Copy link
Member Author

See #9307

@atsansone atsansone added st.WIP Issue in progress infra.structure Relates to the tools that create docs.flutter.dev labels Aug 26, 2023
@AlexV525
Copy link
Member Author

Closing in favor of #9307.

@AlexV525 AlexV525 closed this Aug 27, 2023
@AlexV525 AlexV525 deleted the fix/crlf-to-lf branch August 27, 2023 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra.structure Relates to the tools that create docs.flutter.dev review.tech Awaiting Technical Review st.WIP Issue in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants