arduino_ci icon indicating copy to clipboard operation
arduino_ci copied to clipboard
verified publisher icon Arduino-CI

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

Open hel06492 opened this issue 3 years ago • 8 comments

Highlights from CHANGELOG.md

  • See CHANGELOG.md for more

hel06492 avatar Mar 05 '22 10:03 hel06492

@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!

jgfoster avatar Mar 06 '22 04:03 jgfoster

@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.

hel06492 avatar Mar 07 '22 11:03 hel06492

@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!

jgfoster avatar Mar 07 '22 15:03 jgfoster

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 avatar Mar 07 '22 16:03 ianfixes

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

hel06492 avatar Mar 08 '22 12:03 hel06492

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

ianfixes avatar Mar 08 '22 15:03 ianfixes

@ianfixes yes, it's already checked

hel06492 avatar Mar 09 '22 09:03 hel06492

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

jgfoster avatar Sep 02 '22 02:09 jgfoster

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

ianfixes avatar Dec 17 '22 20:12 ianfixes

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

ianfixes avatar Dec 28 '22 13:12 ianfixes