Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Repo cleanup and improve docs #2656

Merged
merged 3 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,11 @@ Our project is showing it's age and migration across multiple Rails version. Her
```
── app
│   ├── assets
│   │   ├── builds # Old javascript builds - possibly deprecated
│   │   ├── config # Unsure what this is - something PWA related?
│   │   ├── builds # Bundled JavaScript
│   │   ├── config # Specifies assets to be compiled
│   │   ├── fonts
│   │   ├── images
│   │   ├── pdfs # Legacy PDFs from our early years (can be deleted)
│   │   └── stylesheets
│   ├── channels # Unused boilerplate - could be cut
│   ├── components # A mix of mountain_view and view_component components (the latter have the `_component` suffix).
│   ├── constraints # Directs to correct site based on subdomain
│   ├── controllers # Public app controllers
Expand All @@ -140,15 +138,15 @@ Our project is showing it's age and migration across multiple Rails version. Her
│   ├── datatables # Admin area datatables
│   ├── graphql # API
│   ├── helpers
│   ├── javascript # This should be where all the js lives!
│   ├── javascript # Source JavaScript
│   │   ├── 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
Copy link
Collaborator

@lenikadali lenikadali Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

│   └── views
│   ├── admin # Admin area
│   ├── collections # Deprecated feature to create abritrary event collections, was previously used for our early winter festivals
Expand Down Expand Up @@ -183,7 +181,7 @@ Our project is showing it's age and migration across multiple Rails version. Her
│   ├── data # UK geography ward to district data used to create neighbourhood info
│   ├── tasks # Rake tasks that create ActiveJobs
│   └── templates
│   └── erb # Not sure what this is - some kind of template for rails scaffolds?
│   └── erb # Rails scaffold templates
├── log
├── nginx.conf.d # Config files here get added to the nginx config by dokku
├── public
Expand All @@ -209,7 +207,6 @@ Our project is showing it's age and migration across multiple Rails version. Her
│   └── system # Capybara tests for things that need JavaScript
│   ├── admin
│   └── graphql
└── vendor # Currently empty - could potentially delete
```

## API
Expand Down
1 change: 0 additions & 1 deletion app/assets/config/manifest.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//= link_tree ../fonts
//= link_tree ../images
//= link_tree ../pdfs
//= link application.css
//= link_directory ../stylesheets .css
//= link_directory ../stylesheets/themes .css
Expand Down
Binary file removed app/assets/pdfs/2017/12/placecal_winter2017.pdf
Binary file not shown.
Binary file removed app/assets/pdfs/2017/12/placecal_winter2017_bw.pdf
Binary file not shown.
Binary file not shown.
Binary file not shown.
6 changes: 0 additions & 6 deletions app/channels/application_cable/channel.rb

This file was deleted.

6 changes: 0 additions & 6 deletions app/channels/application_cable/connection.rb

This file was deleted.

Empty file removed vendor/assets/javascripts/.keep
Empty file.
Empty file removed vendor/assets/stylesheets/.keep
Empty file.
Loading