search

njh / arduino-libraries

A website listing all the available Arduino Libraries
https://www.arduinolibraries.info/
MIT License
54 stars 12 forks source link

Capitalization of architectures value should not be altered #4

Closed per1234 closed 6 years ago

per1234 commented 7 years ago

The Arduino IDE's handling of the library.properties architectures property is case sensitive (i.e. avr != Avr) but arduinolibraries.info automatically capitalizes the first letter of each item in the "By Architecture" list.

Architecture values with different capitalization should not be combined in this list as they are actually not treated as the same architectures by the Arduino IDE. For example, here we have a library that had the architectures value incorrectly defined as AVR in the latest release: https://github.com/qub1750ul/Arduino_SoftwareReset/blob/v2.0.0/library.properties#L9 while many others correctly have their architectures value set to avr. This should cause your "By Architectures" list to contain an entry for AVR as well as avr yet they are currently combined under Avr.

I have found this project to be extremely useful for easily finding libraries that have errors in their library.properties files so I can submit pull requests to get these issues fixed. Thanks!

njh commented 7 years ago

Hi! I am glad you find the site useful.

I am not sure it would be very useful for (most) users to be able to distinguish between AVR and avr? It wasn't the intension of the developer to make them different things.

Perhaps if I make a custom page, to help you hunt down and fix the bad data?

per1234 commented 7 years ago

I can see your point that combining different capitalizations of architecture values is a reasonable action considering the intended use case ("I'm looking for libraries that run on AVR").

After rejecting that suggestion, an aesthetic issue remains: The automatic capitalization of the first letter of the architecture value just looks wrong to me. I would suggest that it should either be all caps (as is generally the standard for the manufacturer spelling) or all lowercase (which is the convention Arduino has established). I suppose the aesthetic issue is fairly subjective and certainly the current system works well but it's worth considering.

Perhaps if I make a custom page, to help you hunt down and fix the bad data?

That's a very kind offer! I would certainly use it from time to time. Currently I do a quick review of each library when it is submitted for inclusion but that doesn't allow me to catch issues that are introduced in a later release. I also do an occasional search through the JSON file for the common miscapitalizations of the architectures value but that's not very efficient. So I would welcome anything you want to do to make it easier to find problems but of course what you've already done is already a great contribution so whether or not you do anything more I am already grateful.

njh commented 7 years ago

I tried changing them to Uppercase but they looked a bit odd - so I have changed them to lowercase: 29f7457f14672b5fa01145d9bedad638a067a0b1

njh commented 6 years ago

Finally got round to make this page: https://www.arduinolibraries.info/architecture-variants.html

There are an ever increasing number of architectures listed - many with only a small number of libraries.

I guess 'AVR' should be 'avr'? I am not sure why people are using 'atmelavr' - maybe because it is a platform.io value?

per1234 commented 6 years ago

Thanks so much! I used it to find 8 libraries with architecture value case that clearly causes problems. I'm working on PRs to get those fixed right now.

I don't know why they do the atmelavr and atmelsam thing but all of those also use the avr or sam as well so it does no harm.

There is also the stm32 vs. STM32 thing but I found one hardware package that uses each case and have no experience with STM32 so I'll assume they know what they're doing.

I'll go ahead and close this issue now since I think everything we discussed has been resolved.