Aman-Mittal commented on PR #380: URL: https://github.com/apache/fineract-backoffice-ui/pull/380#issuecomment-5311508559
> @Aman-Mittal Would you be able to approve the workflow runs on this one when you get a chance? > > Nothing has executed, so the checks list is empty rather than red, which reads as CI not having started rather than as something waiting on a person. Same gate as #365. > > What is waiting on it, all run on a clean checkout of this branch: `lint`, `lint:prune`, `format:check` and `check-translations` at exit 0, and the unit suite at 1100 of 1100 against real headless Chromium, with the new `clients.routes.spec.ts` at 7 of 7 on its own. The titles were also measured in a browser against the real `en.json`, before and after, which is the table in the description. > > No rush on the review itself. Two things worth flagging while you have this open, though, because one of them is a correction to what I wrote above. > > **The "no new translation keys" claim in the description is too strong.** It is true of `clients` and it is not true in general, so please do not take it as an argument for the convention as stated. Having since worked through `products` and `system`: > > | File | Routes | New keys | > |---|---|---| > | `clients` | 21 | 0 | > | `system` | 49 | 0 | > | `products` | 65 | 6 | > > The accurate version is a rule about route shapes rather than about feature areas. A file needs new keys exactly when it has routes whose heading is bound at runtime. `system` has no `view/:id` route at all, so rule 2 always finds a key across all 49. `products` has five record views, where the visible heading is the record rather than the screen, and those are precisely the routes with no key to reuse. I will write that up properly on #355 once this lands, along with the `tellers` case you asked about. > > **`products` and `system` are already built and held.** Both are committed on local branches, fully verified, and deliberately not pushed. The convention is what this PR is really asking you to agree, and turning up with three PRs before you had answered would have decided it for you. If any of the three rules is wrong, this is the cheapest possible moment to say so and I will redo the other two accordingly. > Thanks, looks ok for me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
