Open ZLLentz opened 1 year ago
A duplicate of https://github.com/pcdshub/pytmc/issues/254 but described here in much better detail.
As you say, we can't really know until the IOC boots up and subs in macros. I'm not sure of a good path forward for this and am definitely open to suggestions.
I searched for old issues but didn't search well enough, whoops
My main ideas are:
So - "if macro included and non-macro part is about 50 chars, warn"?
I think that's pretty reasonable, though I'm not sure how it would get back to the user. Errors are easy because the linter fails with a specific return code in an obvious fashion, whereas warnings require someone to be paying attention to the output even in the case of success.
Integrated with GHA, annotating a specific line wouldn't be easy to implement. Warnings in the summary output would be OK - but would remain somewhat hidden. Hmm...
Maybe simplest is we go around to all the prod configs, see which macros are in use, and then see how long the substitutions typically are. Then in the linter it's straightforward, we automatically sub in $(PREFIX) -> 12 characters or whichever the longest is plus some buffer.
I'd rather this error extra than not enough and PV names at around 57 should still be shortened.
Another option is to allow some level of macro sub length checking based on input args during make
@ZLLentz Why not have the linter check the makfile for macros defined there, and just do a "best I can do" approach? We can't lint custom macros, but the PREFIX is defined in the makefile every time, so if the iocBoot folder exists and there is a makefile and an ioc, could it check the makefile?
This is true in principle, but remember that pytmc is agnostic to your specific ioc/project structure, so we need to be a bit careful in how we implement something like this.
Possibly the simplest is to add a command line argument that ingests macros and apply e.g. PREFIX to it in https://github.com/pcdshub/ioc-common-ads-ioc/blob/81acb0dae4f5e54434d9f3b7d61f3cee7d280aba/iocBoot/templates/Makefile.base#L107-L114
I guess through the above it's worth noting that the length check is actually launched from the aforementioned Makefile, so the info in that Makefile is somewhat trivially fair game to be used as an input.
The following records in my project may or may not be above length 60, but the linter can't actually know because "$(PREFIX)" will be subbed out.
The current behavior is to assume the replaced string is the same length as the macro string. In this case, the prefix is assumed to be 9 characters long. The PLC prefix is actually length 11 in this case, which means the linter check here probably misses additional records that are also above length 60. (In my case, I definitely need to trim some prefix off)
My worry is that for similar cases the linter is failing to catch the maximum length check and letting through some records that will be too long after a macro substitution.