Skip to content

Conversation

jonodrew
Copy link
Contributor

This refactors out a Controller method to a plain ruby object. I'm doing this so that I can access it from other Controllers where we need to update attendance at a Workshop.

It may be that in the future the entire Controller can be removed, but for now I want to make small changes that don't break anything that's already deployed.

@jonodrew
Copy link
Contributor Author

jonodrew commented Aug 19, 2025 via email

@olleolleolle
Copy link
Collaborator

olleolleolle commented Aug 20, 2025

I don't think everyone has to use it - but I will, and it's helpful for me. I don't think it'll impact anyone else on the project, will it?

On 19 August 2025 20:19:04 BST, Till! @.> wrote: @till commented on this pull request. > @@ -0,0 +1,11 @@ +repos: Kinda a no no for me. Would defer to CI for that. 🫣 -- Reply to this email directly or view it on GitHub: #2290 (review) You are receiving this because you authored the thread. Message ID: @.>

I looked, and I learned. cc @till

  • This YAML configuration file is for the Precommit pre-commit.com system. A Python-based, YAML-configured set of git hooks.
  • To actually execute this githook, you'd need to install it on the machine it is to run on: https://pre-commit.com/#installation
    • for users that want it, they run the installation - and enjoy the effects
    • for me, I won't install it, I don't have Python on this machine
    • for CI, it'll use some other system
    • so, in effect, this is a "optional, for users of Precommit, the Python-based system" configuration file. Since it's YAML, perhaps a code comment could explain what is using this file, and how to begin using it, if one so desires.

Note the difference in file location: .git/hooks/precommit vs .pre-commit-config.yaml See https://git-scm.com/book/ms/v2/Customizing-Git-Git-Hooks for details.

@jonodrew
Copy link
Contributor Author

I've updated the commit message to reflect the fact that it's optional

@till
Copy link
Contributor

till commented Aug 20, 2025

I don't think everyone has to use it - but I will, and it's helpful for me. I don't think it'll impact anyone else on the project, will it?

On 19 August 2025 20:19:04 BST, Till! @.> wrote: @till commented on this pull request. > @@ -0,0 +1,11 @@ +repos: Kinda a no no for me. Would defer to CI for that. 🫣 -- Reply to this email directly or view it on GitHub: #2290 (review) You are receiving this because you authored the thread. Message ID: @.>

I looked, and I learned. cc @till

  • This YAML configuration file is for the Precommit pre-commit.com system. A Python-based, YAML-configured set of git hooks.

  • To actually execute this githook, you'd need to install it on the machine it is to run on: https://pre-commit.com/#installation

    • for users that want it, they run the installation - and enjoy the effects

    • for me, I won't install it, I don't have Python on this machine

    • for CI, it'll use some other system

    • so, in effect, this is a "optional, for users of Precommit, the Python-based system" configuration file. Since it's YAML, perhaps a code comment could explain what is using this file, and how to begin using it, if one so desires.

Note the difference in file location: .git/hooks/precommit vs .pre-commit-config.yaml See https://git-scm.com/book/ms/v2/Customizing-Git-Git-Hooks for details.

Thank you, but I already knew.


If we are looking to get to a standard (= how ruby code looks) then I am happy to build a CI job that can run but doesn't fail the build but gives feedback.

Then we can gradually enable rules that may make sense etc..

If this is meant for only one person, then I am not sure that it needs to be checked in. Whatever is checked in becomes status quo usually.

@jonodrew
Copy link
Contributor Author

I'll break this apart. It's clear that the use of pre-commit is more contentious than I anticipated and needs to be discussed somewhere else, but I don't want it blocking the feature development

@jonodrew jonodrew force-pushed the refactor-invites branch 2 times, most recently from 602431d to 31320bb Compare August 20, 2025 07:33
@jonodrew jonodrew requested a review from olleolleolle August 20, 2025 07:43
@jonodrew jonodrew requested a review from olleolleolle August 20, 2025 08:30
This moves the 'attendance management' concerns into a separate object. Having it as a separate object will allow me to update things from elsewhere.

While doing so I've also made a small method to tidy up a long casting.

Signed-off-by: jonathan.kerr <[email protected]>
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.

4 participants