chore: Repo cleanup and improve docs #2656
Conversation
|
sorry huly sync doing weird things |
| │ │ ├── controllers | ||
| │ │ └── src | ||
| │ ├── jobs # Importer logic - jobs are created by cron (`/lib/tasks`). There's a readme here with more info | ||
| │ ├── mailers # Email configuration | ||
| │ ├── models | ||
| │ ├── policies # Pundit rules for who can do and access what | ||
| │ ├── uploaders # CarrierWave rules for handling image and logo uploads | ||
| │ ├── validators # Postcode validator - should possibly live somewhere else | ||
| │ ├── validators # Postcode validator - should possibly live somewhere else, or have other validators moved in here |
There was a problem hiding this comment.
comment: the only other place this kind of code would go (if just the postcode validator is in this folder) is in app/helpers
But this looks like a standard Rails app folder (meant to hold validation code that is used by models in app/models)
There was a problem hiding this comment.
there's a load of stuff here: https://github.com/geeksforsocialchange/PlaceCal/blob/repo-cleanup/app/models/concerns/validation.rb#L15
There was a problem hiding this comment.
Hmm... as I understand it, code in app/concerns would be separate from app/validators. If there's more than enough overlap, I would merge the two (just did a skim; can take a closer look tomorrow with fresher eyes). Otherwise, they can be left as is.
There was a problem hiding this comment.
yeah i think can be left as is for sure its just a bit of a violation of single responsibility principle but theres no hurry to change it
|
Looks good to me ✨ Probably needs a check on staging to make sure we didn't break anything 😅 |
if youre happy plz approve :) if you didnt know, main deploys to placecal-staging.org :) |
Plus this comment (non-blocking, was merely a response to the question 😇) |
Completes easy spring cleaning jobs
Checklist:
Resolves #2636
Description
Motivation and Context
Cleaning up the repo to make it easier to navigate
Type of change
How Has This Been Tested?
N/A
Huly®: PC-2663