Increase Library name length from 16 to 32 #511

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

Jump to bottom

Merged

per1234 merged 5 commits into arduino:main from aliphys:increaseLibNameLengthTo64

Mar 17, 2023

Merged

Increase Library name length from 16 to 32 #511

per1234 merged 5 commits into arduino:main from aliphys:increaseLibNameLengthTo64

Mar 17, 2023

Conversation

@aliphys

Copy link

Contributor

@aliphys aliphys commented Mar 13, 2023 •

edited

Loading

As reported in #508 , the current linter rules limit the max number of characters for a library name to 16.

arduino-lint/etc/schemas/arduino-library-properties-definitions-schema.json

Line 94 in c2c82d6

"maxLength": 16

arduino-lint/internal/rule/ruleconfiguration/ruleconfiguration.go

Line 427 in c2c82d6

MessageTemplate: "library.properties name value {{.}} is longer than the recommended length of 16 characters.",

As noted by @aentinger , since Arduino libraries start with Arduino_, this leaves only 8 characters to describe the library according to the linter rules. This PR increases the maximum length of a library name to ~~(削除) 64 (削除ここまで)~~ 32 characters. For official libraries with the leading Arduino_, this provides ~~(削除) 56 (削除ここまで)~~ 24 characters which should be sufficent to describe the library in a UX-friendly manner.

aliphys added 2 commits

March 13, 2023 18:14

@aliphys


 Change "maxLength" value from 16 to 64

ea63a33

@aliphys


 Update LP010 message

e850295

@aliphys aliphys changed the title ~~(削除) [AE-26] Increase Library name size from 16 to 64 (削除ここまで)~~ (追記) [AE-26] Increase Library name length from 16 to 64 (追記ここまで)

Mar 13, 2023

@aliphys

Copy link

Contributor Author

aliphys commented Mar 13, 2023

@per1234 , I saw you self assigned on #508 .
I increased the character length to 64 in this PR. Could you review it? Thanks :)

@per1234 per1234 self-assigned this

Mar 13, 2023

@per1234 per1234 added type: enhancement topic: code labels

Mar 13, 2023

@per1234 per1234 linked an issue

Mar 13, 2023

that may be closed by this pull request

Change of LP010: Increase character limit for naming Arduino libraries #508

Closed

3 tasks

@aentinger

Copy link

aentinger commented Mar 16, 2023

Honestly, I feel 64 is a bit too much 😉 .

After all we do not want to encourage ThisIsMyFancyArduinoLibraryWhichHasATotalLongAndIncromprehensib(le Name).

I believe 32 would be enough by far.

Secondly, we could add an "exception" for official Arduino libraries, insofar that the "Arduino_" prefix is not counting towards the limit of 16 characters.

@per1234 @aliphys what is your feedback to this?

@aliphys


 32 characters is enough for library name

629e774

@aliphys

Copy link

Contributor Author

aliphys commented Mar 16, 2023

You bring a very compelling example @aentinger . Changed it to 32 characters. 👍

per1234

per1234 requested changes

Mar 16, 2023

View reviewed changes

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.

@aliphys please update the value here as well:

arduino-lint/internal/rule/schema/schemadata/bindata.go

Line 1512 in 629e774

"maxLength": 16

Please update the test data here:

arduino-lint/internal/rule/rulefunction/testdata/libraries/NameGTRecommendedLength/library.properties

Line 1 in 629e774

name=NameGTRecommendedLength

etc/schemas/arduino-library-properties-definitions-schema.json Show resolved Hide resolved

internal/rule/ruleconfiguration/ruleconfiguration.go Show resolved Hide resolved

@per1234

Copy link

Contributor

per1234 commented Mar 16, 2023

@91volt @cmaglie @ubidefeo please discuss the recommended maximum library name length here as requested by @aentinger.

@91volt

Copy link

91volt commented Mar 16, 2023

32 is fine 👍

@cmaglie

Copy link

Member

cmaglie commented Mar 16, 2023

Secondly, we could add an "exception" for official Arduino libraries, insofar that the "Arduino_" prefix is not counting towards the limit of 16 characters.

I agree, but that should be valid also for other vendors, like Adafruit_, SparkFun_, ????_... which have a lot of libraries.

Anyway, I guess that just increasing the limit to 32 should already alleviate it.

@aentinger

Copy link

aentinger commented Mar 17, 2023

Don't get me wrong, "36" is fine too. I was just baffled to be presented with this number out of the blue. If 36 makes sense because its the max amount of chars that can be sensibly displayed by the IDE then this is a reasonable criteria imho.

I agree, but that should be valid also for other vendors, like Adafruit_, SparkFun_, ????_... which have a lot of libraries.

Fair enough 😁 . It was just a thought. Given that it is "just" a warning leaving it at 16 is also fine, but I think you all know about the dangers of ignoring warnings and normalization of deviance. Both 36 or 32 would alleviate that pain point for all involved. I'd be fine with going ahead with 36.

@cmaglie

Copy link

Member

cmaglie commented Mar 17, 2023

LOL, ok, let's not get this longer than needed...

I decide: let's go with 32 :-)

@aliphys


 Change schema maxLength to 32

4ea2ce2

@aliphys aliphys force-pushed the increaseLibNameLengthTo64 branch from 59a92d9 to 4ea2ce2 Compare

March 17, 2023 12:06

@codecov-commenter

Copy link

codecov-commenter commented Mar 17, 2023 •

edited

Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (c2c82d6) 90.05% compared to head (b0f7c8a) 90.05%.

Additional details and impacted files

@@ Coverage Diff @@
## main #511 +/- ##
=======================================
 Coverage 90.05% 90.05% 
=======================================
 Files 44 44 
 Lines 6800 6800 
=======================================
 Hits 6124 6124 
 Misses 553 553 
 Partials 123 123

Flag	Coverage Δ
unit	`90.05% <ø> (ø)`

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

Impacted Files	Coverage Δ
...ternal/rule/ruleconfiguration/ruleconfiguration.go	`100.00% <ø> (ø)`
internal/rule/schema/schemadata/bindata.go	`49.67% <ø> (ø)`

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@aliphys


 Change test name to NameGTRecommendedLength

b0f7c8a

@aliphys

Copy link

Contributor Author

aliphys commented Mar 17, 2023

Since we decided together on 32 characters as the max library length 🎉 , I changed NameGTRecommendedLength (23 characters) to NameIsBiggerThanRecommendedLength (33 characters).

@aliphys

Copy link

Contributor Author

aliphys commented Mar 17, 2023

For the record, Codecov is really useful for the CI/CD testing pipeline. When I changed the schema file (b0f7c8a) to 32 it reported a loss in code coverage.
image
This was then resolved immediately after b0f7c8a
image

😄

@aliphys aliphys requested a review from per1234

March 17, 2023 12:26

@aliphys aliphys changed the title ~~(削除) [AE-26] Increase Library name length from 16 to 64 (削除ここまで)~~ (追記) [AE-26] Increase Library name length from 16 to 32 (追記ここまで)

Mar 17, 2023

per1234

per1234 approved these changes

Mar 17, 2023

View reviewed changes

@per1234 per1234 merged commit 36204a0 into arduino:main

Mar 17, 2023

@aliphys aliphys deleted the increaseLibNameLengthTo64 branch

April 11, 2023 18:47

@aliphys aliphys mentioned this pull request

Aug 24, 2023

[AE-158] Arduinofy Library Name arduino-libraries/Arduino_UnifiedStorage#1

Merged

@aliphys aliphys mentioned this pull request

Sep 4, 2023

Add Arduino_POSIXStorage to repositories.txt arduino/library-registry#3289

Merged

@aliphys aliphys mentioned this pull request

Sep 7, 2023

[AE-158] Add Arduino_UnifiedStorage to repositories.txt arduino/library-registry#3300

Merged

@per1234 per1234 changed the title ~~(削除) [AE-26] Increase Library name length from 16 to 32 (削除ここまで)~~ (追記) Increase Library name length from 16 to 32 (追記ここまで)

Oct 23, 2023

Labels

topic: code type: enhancement

6 participants

@aliphys @aentinger @per1234 @91volt @cmaglie @codecov-commenter

Uh oh!

Increase Library name length from 16 to 32 #511

Increase Library name length from 16 to 32 #511

Uh oh!

Conversation

@aliphys aliphys commented Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aliphys commented Mar 13, 2023

Uh oh!

aentinger commented Mar 16, 2023

Uh oh!

aliphys commented Mar 16, 2023

Uh oh!

@per1234 per1234 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

per1234 commented Mar 16, 2023

Uh oh!

91volt commented Mar 16, 2023

Uh oh!

cmaglie commented Mar 16, 2023

Uh oh!

aentinger commented Mar 17, 2023

Uh oh!

cmaglie commented Mar 17, 2023

Uh oh!

codecov-commenter commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aliphys commented Mar 17, 2023

Uh oh!

aliphys commented Mar 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

@aliphys aliphys commented Mar 13, 2023 •

edited

Loading

codecov-commenter commented Mar 17, 2023 •

edited

Loading