Asana / locheck

Validate iOS, Android, and Mac localizations. Find errors in .strings, .stringsdict, and strings.xml files.
MIT License
96 stars 10 forks source link

Allow treating warnings as errors #26

Closed jklausa closed 2 years ago

jklausa commented 2 years ago

Heyo :)

I love that you wrote this (and open-sourced!), since it solved a big pain point I've had before with localisation tools not being harsh enough in validation.

Would you consider adding a semi-stable (it's before 1.0, so... whatever goes) output format that could be parsed by other tools? My use-case is to have locheck be part of our CI suite, so we can make sure the strings we're merging are correct, but it's currently not super nice for that use-case. (I'm currently parsing the stdout output, and looking for lines that begin with "WARNING: or ERROR:, which, workable, but... yeah.)

JSON would probably be the easiest?

And I know that the README says:

Pull requests for additional output formats will probably be accepted.

but I wanted to check in with y'all if this is something you'd be even interested in / maybe are working on already, before I start hacking on it.

irskep commented 2 years ago

We are interested, but are waiting for someone to ask for a specific format. "JSON" is too broad because the schema can be anything. We only use the xcodebuild output format internally. Does your CI suite have an existing XML or JSON schema it uses to report errors?

jklausa commented 2 years ago

We only use the xcodebuild output format internally.

I'm guessing/assuming you mean you're using this as a "Build Phase" script? That means incurring a time penalty for each build, even when you didn't touch strings, right?

Does your CI suite have an existing XML or JSON schema it uses to report errors?

Not really, we just examine the CI logs to see what's broken.

I don't really need anything fancy here, I just want something I can parse in Ruby and then either fail the CI run by exit(1)ing, or report a success.

irskep commented 2 years ago

Yes, we use it as a build phase, but only in CI, not locally. We also just read the logs to see what broke—it's why I added the 'summary' feature on stdout. If it fails we can quickly see which strings need to be fixed without trying to read the xcodebuild output.

You don't need to parse the output to check for success or failure, just look for a nonzero exit code. If the exit code is zero, there were no errors, only warnings. If there were errors, the stdout summary should have everything you need.

If you would prefer that warnings also trigger a nonzero exit code, we should add a flag for that, not a new output format. :-)

jklausa commented 2 years ago

If you would prefer that warnings also trigger a nonzero exit code, we should add a flag for that, not a new output format. :-)

You know what? I'm 100% sure I had that thought at one point when working on this, but then I was like "huh, I wonder if I can hack it somehow", ended up doing some weird parsing stuff, and then my mind went to "sure would be nice to be able to parse that more easily!"; instead of circling back to "I want warnings as errors".

Should I open another issue for --treat-warnings-as-errors or whatever ;P?

irskep commented 2 years ago

Yes, that sounds good. It should be extremely easy to implement and you're welcome to take a crack at it, since I won't be working on this for at least another 3-4 weeks.

jklausa commented 2 years ago

Things got reshuffled for me at $dayJob and I didn't have time to work on this yet, but I'll take a stab at it this week :)

stevelandeyasana commented 2 years ago

Done! #28.

stevelandeyasana commented 2 years ago

This change has been released as part of 0.9.4.