Skip to content

[breaking] Improved .pde warnings detection subroutines / gRPC LoadSketch API improvement #2490

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 10 commits into from
Jan 8, 2024

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Jan 5, 2024

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

  • The .pde deprecation warning is printed only when the sketch is valid.
  • The scanning for pde files has been optimized
  • The gRPC API about LoadSketch has been improved as well as part of the refactoring.

What is the current behavior?

$ arduino-cli compile -b arduino:avr:uno
Sketches with .pde extension are deprecated, please rename the following files to .ino:
 - /home/megabug/Workspace/arduino-cli/internal/arduino/builder/testdata/TestLoadSketchFolder/old.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/builder/testdata/TestLoadSketchFolderBothInoAndPde/TestLoadSketchFolderBothInoAndPde.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/builder/testdata/TestLoadSketchFolderBothInoAndPde/old.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/builder/testdata/TestLoadSketchFolderPde/TestLoadSketchFolderPde.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/builder/testdata/TestLoadSketchFolderPde/old.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/old.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/libraries/testdata/TestLibExamples/examples/MultipleFiles/MultipleFiles.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/sketch/testdata/SketchBothInoAndPde/SketchBothInoAndPde.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/sketch/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/sketch/testdata/SketchPde/SketchPde.pde
 - /home/megabug/Workspace/arduino-cli/internal/arduino/sketch/testdata/SketchSymlinkSrc/old.pde
 - /home/megabug/Workspace/arduino-cli/internal/integrationtest/compile_4/testdata/SketchWithMergedSketchAndBootloader/old.pde
 - /home/megabug/Workspace/arduino-cli/internal/integrationtest/testdata/sketch_multiple_main_files/sketch_multiple_main_files.pde
 - /home/megabug/Workspace/arduino-cli/internal/integrationtest/testdata/sketch_pde_main_file/sketch_pde_main_file.pde
 - /home/megabug/Workspace/arduino-cli/internal/integrationtest/testdata/sketch_simple/old.pde
Can't open sketch: main file missing from sketch: /home/megabug/Workspace/arduino-cli/arduino-cli.ino
$

The pde warnings are printed even if the current directory is not a valid sketch.

What is the new behavior?

$ arduino-cli compile -b arduino:avr:uno
Can't open sketch: main file missing from sketch: /home/megabug/Workspace/arduino-cli/arduino-cli.ino
$

Does this PR introduce a breaking change, and is titled accordingly?

Yes, there is a breaking change in the gRPC API.

Other information

Fix #2384

@cmaglie cmaglie added type: enhancement Proposed improvement topic: CLI Related to the command line interface topic: gRPC Related to the gRPC interface labels Jan 5, 2024
@cmaglie cmaglie added this to the Arduino CLI v1.0.0 milestone Jan 5, 2024
@cmaglie cmaglie self-assigned this Jan 5, 2024
@cmaglie cmaglie force-pushed the reduced_pde_warnings branch from c7d1283 to 64e5234 Compare January 5, 2024 16:28
@cmaglie cmaglie force-pushed the reduced_pde_warnings branch from 363e41c to 173f8bc Compare January 5, 2024 19:47
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (95753fc) 68.85% compared to head (173f8bc) 68.80%.

Files Patch % Lines
internal/cli/upload/upload.go 50.00% 2 Missing and 1 partial ⚠️
commands/instances.go 50.00% 1 Missing and 1 partial ⚠️
internal/cli/arguments/sketch.go 66.66% 1 Missing and 1 partial ⚠️
commands/daemon/daemon.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2490      +/-   ##
==========================================
- Coverage   68.85%   68.80%   -0.05%     
==========================================
  Files         204      204              
  Lines       20465    20443      -22     
==========================================
- Hits        14091    14066      -25     
- Misses       5220     5222       +2     
- Partials     1154     1155       +1     
Flag Coverage Δ
unit 68.80% <89.74%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmaglie cmaglie merged commit ea8c281 into arduino:master Jan 8, 2024
@cmaglie cmaglie deleted the reduced_pde_warnings branch February 8, 2024 11:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
topic: CLI Related to the command line interface topic: gRPC Related to the gRPC interface type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warnings for .pde file are emitted even if the current dir is not a sketch
2 participants