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

Switch to using double quotes by default #168

Closed
wants to merge 14 commits into from
Closed

Conversation

dgmstuart
Copy link
Owner

I give up: I wanted to try to use the "community default", but all my
tooling (snippets etc) is set up using double quotes, so it's just
friction which is annoying.

Anyway it seems like the community is far from being in agreement on
this topic:

rubocop/rubocop#5306

protect_from_forgery is on by default from rails 5.2 onwards.

MD5 was super old: added when I was still using BasicAuth back in
4e4e964

No idea if sign_out ever did anything
On SOLDN time, it's still today until 4am tomorrow. This feature was
added in order to avoid events disappearing at midnight, since events
sometimes finish quite late.

But it was implemented very messily, and before we had some of the
better date support we have in Rails now.

The extra timezone testing is probably unnecessary, but I want to be
sure that this doesn't behave oddly in the summertime.

Note we need to add an inflection to allow Zeitwerk to autoload this
class.
We have Date#current, so this isn't needed.
Let's control this a little more: we expose the _method_ to controllers
at the top level, but leave it to individual controllers to decide what
they do with it.
This probably wasn't working (I only just reinstated analytics) and it
looks like in GA4 this is an admin setting in the UI which is turned on
by default:

  https://analytics.google.com/analytics/web/#/a18613144p359226015/admin/streams/table/4755631820
We're not using them on the public pages.
There's a bit of duplication here, but I think it's worth it to keep a
clear separation of user-facing vs admin.
This is mostly a lot of renaming.

Note that the main partials from the listings were previously namespaced
under /events

Trash the website controller spec: it's almost useless and was done
before I had any clue what testing was.
Not sure why these started breaking - it was after a big renaming so
perhaps they end up just being called in a different order? Either way I
guess we're expecting them to fail, since lib classes aren't required by
default.
This is almost a duplicate of the cms layout, but it's pretty simple so
it's easy to reverse this decision later.
Putting them in the page itself was a hack.
Now the application layout is used for only user-facing pages, we can
put analytics on all those pages, and use the stylesheet on all the
pages.
I give up: I wanted to try to use the "community default", but all my
tooling (snippets etc) is set up using double quotes, so it's just
friction which is annoying.

Anyway it seems like the community is far from being in agreement on
this topic:

  rubocop/rubocop#5306
@dgmstuart dgmstuart closed this Mar 26, 2023
@dgmstuart dgmstuart deleted the refactor/quotes branch March 26, 2023 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant