Skip to content

Conversation

Bikappa
Copy link
Contributor

@Bikappa Bikappa commented Feb 9, 2023

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)

What kind of change does this PR introduce?

Provides a json schema for the cli configuration file, for now the schema is not used by the runtime itself and it's only meant for users initiatives.

A possible followup of this pr is the autogeneration of the list of options in configuration.md

What is the current behavior?

What is the new behavior?

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

Other information

Fixes #587

@Bikappa Bikappa self-assigned this Feb 9, 2023
@Bikappa Bikappa added the topic: documentation Related to documentation for the project label Feb 9, 2023
@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Base: 36.62% // Head: 36.56% // Decreases project coverage by -0.07% ⚠️

Coverage data is based on head (46fadf1) compared to base (940c945).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2068      +/-   ##
==========================================
- Coverage   36.62%   36.56%   -0.07%     
==========================================
  Files         229      229              
  Lines       19539    19539              
==========================================
- Hits         7156     7144      -12     
- Misses      11547    11556       +9     
- Partials      836      839       +3     
Flag Coverage Δ
unit 36.56% <ø> (-0.07%) ⬇️

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

Impacted Files Coverage Δ
arduino/monitor/monitor.go 41.05% <0.00%> (-6.32%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Bikappa Bikappa marked this pull request as draft February 9, 2023 08:59
@Bikappa Bikappa force-pushed the chore/json-schema branch 9 times, most recently from 724b3e3 to d012145 Compare February 9, 2023 10:16
@Bikappa Bikappa marked this pull request as ready for review February 9, 2023 11:11
@Bikappa Bikappa requested review from umbynos and per1234 February 9, 2023 12:22
@per1234 per1234 added the type: enhancement Proposed improvement label Feb 11, 2023
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

UPDATE: Resolved by 8ad388e

Please add a mention of and link to the schema in the documentation of the configuration file:

https://github.com/arduino/arduino-cli/blob/chore/json-schema/docs/configuration.md#configuration-file

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

UPDATE: superseded by 99eb639

Since it was not feasible to do via the PR review system, I also submitted a PR against this PR's branch for an additional suggestion: https://github.com//pull/2073

@Bikappa Bikappa force-pushed the chore/json-schema branch 3 times, most recently from 7134c2a to 34c68ce Compare February 16, 2023 13:01
Co-authored-by: per1234 <accounts@perglass.com>
@Bikappa
Copy link
Contributor Author

Bikappa commented Feb 16, 2023

Since it was not feasible to do via the PR review system, I also submitted a PR against this PR's branch for an additional suggestion: #2073

As discussed with @per1234 a unit test was introduced instead

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Thanks Luca!

@Bikappa Bikappa merged commit c95f890 into master Feb 22, 2023
@Bikappa Bikappa deleted the chore/json-schema branch February 22, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Related to documentation for the project type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[proposal] Please provide us a JSON schema for the new arduino-cli.yaml config file
3 participants