RFC-175: Reduce number of frontend apps #175
Conversation
KludgeKML
changed the title
KludgeKML
force-pushed
the
frontend-fewer-apps
branch
from
107e9dc to
431d446
Compare
|
|
||
| There are currently 8 rendering apps for GOV.UK that are supported and maintained by GOV.UK teams, 4 of which are almost exclusively concerned with rendering content: | ||
|
|
||
| 1. [collections] |
There was a problem hiding this comment.
I don't think there's any reason this couldn't be merged to frontend so it's a bit out of scope of the RFC really, but do you see the merged frontend app as the long term home of the organisations API, which currently resides in collections?
There was a problem hiding this comment.
What currently uses that API?
There was a problem hiding this comment.
A fair few things, although I think a lot of them are no longer active. Manuals Publisher and Short URL Manager are though
There was a problem hiding this comment.
Ah, I see it's here: https://github.com/alphagov/collections/blob/main/docs/api.md - mainly internal apps. Good question! It's probably not a terrible place for it to be, given that it's publicly available information and other people might potentially use it.
There was a problem hiding this comment.
In some ways, the merged app would become a more appropriate place for it, because it's somewhat like the search results in Finder Frontend. Perhaps Search API is a natural home? I think it's a tricky question, because there's no particular good home for it, so maybe for the moment it makes as much sense to keep it here as anywhere. But when the routes are moved, we'll consider if there's a better place - we don't want to treat the grouping of the routes in the four current apps as handed down to us on stone tablets.
rfc-175-frontend-fewer-apps.md
Outdated
|
|
||
| ## Summary | ||
|
|
||
| This is a refinement of the suggestions in [RFC-174] about reducing the number of frontend rendering applications, reducing its scope to the applications that deal exclusively with rendering content items. By dealing only with the four applications that exclusively render content items, we improve standardisation and reduce maintenance costs, and the work is worthwhile even if we can only achieve sections of it. Because a large part of the problem and a lot of the solution remains the same, we have divided this RFC into two parts - an initial section that describes how we differ from [RFC-174], and a more verbose section detailing the approach. If you have already read or commented on [RFC-174], you can read the [How this proposal differs](#how-this-proposal-differs) section and skip the rest of the RFC. If you are new to this proposal, feel free to skip to the [Full Exploration of the Problem](#full-exploration) section. |
There was a problem hiding this comment.
Is it worth being explicit that RFC-174 was closed and not accepted? The language in this RFC made me assume 174 had been merged and then descoped here in 175.
There was a problem hiding this comment.
Ah yes, good point - and we should properly merge 174 in marked as rejected(? or whatever we do with rejected RFCs)
There was a problem hiding this comment.
Ah, we don't merge in, I was confused by the status of some of the merged RFCs. Okay I just need to make explicit that RFC 174 wasn't accepted.
There was a problem hiding this comment.
This sounds great @KludgeKML, thanks for putting this together. I really looking forward to seeing this come together, it feels like working on GOV.UK would be so much easier with these things combined.
rfc-175-frontend-fewer-apps.md
Outdated
| Under `/config` | ||
|
|
||
| `data` | ||
| - YAML, JSON, CSV config files. Data files should be placed here rather than under `/lib/data`. |
There was a problem hiding this comment.
In the time since the last RFC we on the AI team have had to deal in anger with considering the place for data files and found the config/data route was actually a bit surprising and lib/data was a better choice.
Our rationale was:
- when people tend to be reaching for a
datadirectory it tends to not actually be things that configure the app - so config/data seemed surprising for something that isn't doing any configuration - when putting things into the config directory you would typically want them to be accessible via
Rails.configurationto be conventional config, and it seems strange to potentially have a directory of files that isn't - Rails already has the precedent for non namespaced storage in lib, with the assets and tasks precedence. It's really easy to flag a directory to be ignored by the autoloader: https://github.com/alphagov/govuk-chat/blob/54059d50aff6211328fb4f8302333adaaa03b02d/config/application.rb#L26-L29
The tricky bit is the blurring of the line between what is config and what is data - which you'll probably have even with this decision to choose between whether a file lives in config or config/data. The barometer we found worked well was whether it makes sense to be part of Rails.configuration
There was a problem hiding this comment.
Hmm. Looking at the current use of lib/data, only frontend uses it really (two apps use it for recruitment banner config, but as per RFC-176 we're looking to remove that code in app anyway), and it uses it to hold two content items that it publishes. (bank holiday and clock change content items). Given that those should probably not be in Frontend anyway, I think this might be a moot point in practice. In theory, though, we could have things in both directories depending on what they were?
There was a problem hiding this comment.
whether it makes sense to be part of
Rails.configuration
I think that statement is open to a lot of interpretation too.
There doesn't seem to be a consensus in the wider world about where to data files either. It might be better to have a top level /data directory for model data.
There was a problem hiding this comment.
I've added a note here that /config/data should be for data that's clearly for configuration, a root data directory /data for model data, but that in general we should try to avoid using data files for configuration in these apps.
rfc-175-frontend-fewer-apps.md
Outdated
|
|
||
| Under `/config` | ||
|
|
||
| `data` |
There was a problem hiding this comment.
Sorry to nitpick further, if you've got a separate /data directory surely you'd want this config just in /config directly - shouldn't be any need for a separate data directory.
rfc-175-frontend-fewer-apps.md
Outdated
| `locales` | ||
| - Static content like lists of contact information should ideally be stored in the locale files to promote localisation. | ||
|
|
||
| Under `/data` |
There was a problem hiding this comment.
Nitpicking further but this seems less conventional than using lib/data since you'll be adding a new top level directory, whereas lib is already intended to be expanded
There was a problem hiding this comment.
There's no defined logical place to define it. Personally I find it odd that the data files are in /lib/data but the models that use them are in app, but then the data schema is in /db so perhaps /data is ok? I'd suggest /app/data but that seems even more unconventional.
Hopefully this will all be a moot point because we want to get away from having modelled data in the frontend apps, unless there's another postcode restrictions situation. In that example we did put the restrictions data in config.
There was a problem hiding this comment.
Yeah sounds good about it being a moot point - there's something smelly about a directory named data, reminds me of other dubious ones like "shared" and "util".
I think the main thing is to avoid there being two data directories - one under /config/data and another one as this introduces further ambiguity.
I do agree that there is not a formally defined location in Rails, so does veer into the realm of individual interpretations of the enigmatic "Rails way". Would also agree that app/data is the most surprising suggestion so far. I wouldn't share your concern that it's weird something in app referencing a resource from lib but I know that veers into the other great Rails debate about the relationship of those directories.
There was a problem hiding this comment.
This is great @KludgeKML
Sorry that my last comments seem to have been on about the most minor of minor things in this - I really did want to just let it go but felt a bit guilty that I'm worried my last comments have led to you actually proposing something slightly worse than your initial proposal. Might be best to just take the data stuff out since it's so tangential to the actual core of this proposal?
No need to apologise! It's better to bring up small points when we're being prescriptive. I'm off until Tuesday now, but perhaps @leenagupte can do the honours? (Perhaps it's worth leaving in that we'll try to avoid data files where possible without defining exactly where we'll avoid them 😉) |
In Keith's absence and to get this RFC over the line I have updated the RFC to reflect that the data files if they exist should be co-located, but the exact location is yet to be determined.
There was a problem hiding this comment.
It looks like all of the concerns raised in the comments have been addressed.
The only minor issue left is about where to put data files. I think that there was agreement that these types of files shouldn't exist if possible, and if they do, they should all be located in the same place.
I think it's ok for this RFC to be accepted with the decision of where exactly to put data files left for another day, as the existing files are due to be removed.
Refinement of RFC-174 with reduced scope, proposing merging: collections, finder-frontend, and government-frontend into the frontend app.
Rendered version: https://github.com/alphagov/govuk-rfcs/blob/frontend-fewer-apps/rfc-175-frontend-fewer-apps.md
Closing date for comments: Thu 22nd August 2024
https://trello.com/c/54amoVVe/242-draft-new-reduced-scope-rfc-for-consolidating-apps