Skip to content

Fix the outpit directory for ktlint#3541

Merged
auto-submit[bot] merged 2 commits intoflutter:mainfrom
gmackall:fix_output_dir_ktlin
Mar 4, 2024
Merged

Fix the outpit directory for ktlint#3541
auto-submit[bot] merged 2 commits intoflutter:mainfrom
gmackall:fix_output_dir_ktlin

Conversation

@gmackall
Copy link
Member

@gmackall gmackall commented Mar 4, 2024 •
edited
Loading

In #3522 I landed a build.sh which uses curl with the output

 -o $DIR/../build/ktlint/

I didn't know -o is intended to target the output file, not directory (both the file and the directory in this case are named ktlint). Hence the warning:

Warning: Failed to create the file 
Warning: /b/s/w/ir/x/w/cocoon/cipd_packages/ktlint/tools/../build/ktlint: Is a 
Warning: directory

Changes to -o $DIR/../build/ktlint/ktlint

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@gmackall gmackall requested a review from keyonghan March 4, 2024 20:03
@gmackall gmackall marked this pull request as ready for review March 4, 2024 20:03
keyonghan
keyonghan approved these changes Mar 4, 2024
View reviewed changes
Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM

@gmackall
Copy link
Member Author

gmackall commented Mar 4, 2024

Sorry, I just realized I think that the other packages aren't building their output at <build>/<packagename>/<packagename>, but rather just at <build>/<packagename>, so I changed the PR to instead just not make build/ktlint a directory (and then output the file there).

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 4, 2024
@keyonghan
Copy link
Contributor

keyonghan commented Mar 4, 2024

Sorry, I just realized I think that the other packages aren't building their output at <build>/<packagename>/<packagename>, but rather just at <build>/<packagename>, so I changed the PR to instead just not make build/ktlint a directory (and then output the file there).

It depends on how people reference the packages and their contents. From CI side, either way works, whenever the recipes specify the correct paths.

@gmackall gmackall removed the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 4, 2024
@gmackall
Copy link
Member Author

gmackall commented Mar 4, 2024

Sorry, I just realized I think that the other packages aren't building their output at <build>/<packagename>/<packagename>, but rather just at <build>/<packagename>, so I changed the PR to instead just not make build/ktlint a directory (and then output the file there).

It depends on how people reference the packages and their contents. From CI side, either way works, whenever the recipes specify the correct paths.

Got it, thanks for the clarification!

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 4, 2024
@auto-submit auto-submit bot merged commit bd1ec4e into flutter:main Mar 4, 2024
sealesj pushed a commit to sealesj/cocoon that referenced this pull request Mar 7, 2024
@gmackall @sealesj
Fix the outpit directory for ktlint (flutter#3541)
0ea3a97 
In flutter#3522 I landed a `build.sh` which uses `curl` with the output
```
 -o $DIR/../build/ktlint/
```
I didn't know `-o` is intended to target the output file, not directory (both the file and the directory in this case are named ktlint). Hence the warning:
```
Warning: Failed to create the file 
Warning: /b/s/w/ir/x/w/cocoon/cipd_packages/ktlint/tools/../build/ktlint: Is a 
Warning: directory
```

Changes to `-o $DIR/../build/ktlint/ktlint`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@keyonghan keyonghan keyonghan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gmackall @keyonghan