Skip to content

dist/tools: add sort-variables script#6818

Closed
vincent-d wants to merge 1 commit intoRIOT-OS:masterfrom
OTAkeys:pr/tool_sort_variables
Closed

dist/tools: add sort-variables script#6818
vincent-d wants to merge 1 commit intoRIOT-OS:masterfrom
OTAkeys:pr/tool_sort_variables

Conversation

@vincent-d
Copy link
Copy Markdown
Member

This is the script I used for #6740.

It's not really tested and comes without any warranty as it might have some side effects.

I thought it might still be interesting to keep it, though.

@vincent-d vincent-d added the Area: tools Area: Supplementary tools label Mar 30, 2017
@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 31, 2017

Would be great if this tool could also check if the variables are sorted.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 31, 2017

(for CI)

@miri64 miri64 self-assigned this Apr 6, 2017
@miri64
Copy link
Copy Markdown
Member

miri64 commented Apr 6, 2017

@vincent-d ping?

@vincent-d
Copy link
Copy Markdown
Member Author

Sorry, this was not in my priority list ;)

I don't know if I'll have time to work on this soon, but feel free to improve if you want.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Apr 25, 2017

Okay, but I guess it is a start :-)

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 25, 2017
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Works like a charm. ACK.

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Wait...

dist/tools/sort-variables/sort-variables.sh . BOARD_BLACKLIST

yields (among other things):

diff --git a/examples/dtls-echo/Makefile b/examples/dtls-echo/Makefile
index 25b97f6..d53d3f8 100644
--- a/examples/dtls-echo/Makefile
+++ b/examples/dtls-echo/Makefile
@@ -8,10 +8,9 @@ BOARD ?= native
 RIOTBASE ?= $(CURDIR)/../..
 
 # TinyDTLS only has support for 32-bit architectures ATM
-BOARD_BLACKLIST := arduino-duemilanove arduino-mega2560 arduino-uno chronos \
-                   msb-430 msb-430h telosb waspmote-pro wsn430-v1_3b wsn430-v1_4 \
-                   z1 \
-                   nrf52dk # see #6022
+BOARD_BLACKLIST := # #6022 arduino-duemilanove arduino-mega2560 arduino-uno chronos \
+                   msb-430 msb-430h nrf52dk see telosb waspmote-pro wsn430-v1_3b \
+                   wsn430-v1_4 z1
 
 BOARD_INSUFFICIENT_MEMORY := airfy-beacon calliope-mini cc2650stk maple-mini \
                              microbit nrf51dongle nrf6310 nucleo32-f031 \

So this is broken for comments :-/

@vincent-d
Copy link
Copy Markdown
Member Author

It can be easy to remove comments before sorting, but I don't see an easy way to sort and keep comments...

@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 25, 2017
if [ $# -lt 2 ]; then
echo "Usage: sort-variables.sh <path> <variable>"
echo "Sort the <variable> alphabetically, and cut lines to 80 characters"
echo "It will search is <path> and its subdirectories"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"is" superflous?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

or should be in?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 15, 2017

Any plans to work on this further? Maybe we can come up with a solution to keep the comments or agree on a scheme how to comment on a list otherwise.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 15, 2018

Ping @vincent-d?

@vincent-d vincent-d removed this from the Release 2018.04 milestone Apr 11, 2018
@tcschmidt
Copy link
Copy Markdown
Member

@vincent-d how would you like to proceed here? This seems to require some strategic effort ;)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 9, 2019

Ping @vincent-d. Can this be closed in favor of #10111?

@stale
Copy link
Copy Markdown

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 9, 2019

ping @vincent-d again

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Sep 9, 2019
@vincent-d
Copy link
Copy Markdown
Member Author

Sorry for being unresponsive here.

I still don't have time to invest in this. As I said, I guess we could remove comments when sorting. If everyone can live with this, then we could merge.

@stale
Copy link
Copy Markdown

stale bot commented Mar 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 13, 2020
@stale stale bot closed this Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: tools Area: Supplementary tools State: stale State: The issue / PR has no activity for >185 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants