Skip to content

Remove flag --no-dry-run for arduino-cli >= 0.14.0 #323

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

Closed
wants to merge 3 commits into from

Conversation

hel06492
Copy link

@hel06492 hel06492 commented Mar 5, 2022

Highlights from CHANGELOG.md

  • See CHANGELOG.md for more

@jgfoster
Copy link
Member

jgfoster commented Mar 6, 2022

@hel06492, Thank you for this PR. Could you give us a bit of background on why it is desirable or what problem it solves? Next, why tie it to an arduino_ci version? If it is desirable, why not have it take effect immediately? Presumably people using the old version will already have the legacy behavior. Thanks!

@hel06492
Copy link
Author

hel06492 commented Mar 7, 2022

@jgfoster The --dry-run flag was removed from version 0.14.0 and as for version 0.19.0 arduino-cli no longer accepts the flag. It fails with the following error. I can't find the changelog, only a forum post about the change.

Compiling Blink.ino for arduino:avr:mega:cpu=atmega2560... 
Last command:  $  /usr/bin/arduino-cli --format json compile --fqbn arduino:avr:mega:cpu=atmega2560 --warnings all --dry-run /home/salaryman/Arduino/trial.errors/Blink/examples/Blink/Blink.ino
Error: unknown flag: --dry-run

The version check is for older versions which might give unexpected behavior without the --dry-run flag.

Please let me know if there are more things I need to fix/add for this pull request. Thanks.

@jgfoster
Copy link
Member

jgfoster commented Mar 7, 2022

@hel06492, I think some of my confusion is that the change refers to arduino_ci (Continuous Integration) while it appears that it should refer to arduino_cli (Command Line Interface). Could you update the title and other text in the PR and comments in the code to make that clarification? Thanks!

@ianfixes
Copy link
Collaborator

ianfixes commented Mar 7, 2022

Hi @hel06492, nice work on the ruby side of things! It looks like this relates to #277 👍

Please let me know if there are more things I need to fix/add for this pull request.

I only found one issue. In general, I would welcome contributions that upgrade DESIRED_ARDUINO_CLI_VERSION to the latest.

@ianfixes ianfixes changed the title Remove flag --no-dry-run for arduino_ci >= 0.14.0 Remove flag --no-dry-run for arduino-cli >= 0.14.0 Mar 7, 2022
@hel06492
Copy link
Author

hel06492 commented Mar 8, 2022

Hi @jgfoster @ianfixes
Thanks for the suggestions. I've made changes according to those feedbacks.

@ianfixes
Copy link
Collaborator

ianfixes commented Mar 8, 2022

@hel06492 Have you selected "allow edits from maintainers" on your PR? I can probably take care of the remaining issues.

@hel06492
Copy link
Author

hel06492 commented Mar 9, 2022

@ianfixes yes, it's already checked

@jgfoster
Copy link
Member

jgfoster commented Sep 2, 2022

@ianfixes, I'm going through the open PRs and wonder if this can be merged. Thanks!

@ianfixes
Copy link
Collaborator

ianfixes commented Dec 17, 2022

I've merged this with a larger body of work that will be merged as part of #334 -- thanks!

@ianfixes ianfixes closed this Dec 17, 2022
@ianfixes
Copy link
Collaborator

This has merged as part of #334 and was released in v1.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants