FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Refactor invites by jonodrew · Pull Request #2290 · codebar/planner · GitHub

Refactor invites - #2290

Open
jonodrew wants to merge 1 commit into
codebar:masterfrom
jonodrew:refactor-invites
Open

jonodrew wants to merge 1 commit into
codebar:masterfrom
jonodrew:refactor-invites

Conversation

Copy link
Copy Markdown
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.

Comment thread .pre-commit-config.yaml Outdated

jonodrew commented Aug 19, 2025 via email

Copy link
Copy Markdown
Contributor Author

olleolleolle commented Aug 20, 2025
edited
Loading

Copy link
Copy Markdown
Collaborator

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.

Copy link
Copy Markdown
Contributor Author

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

till commented Aug 20, 2025

Copy link
Copy Markdown
Collaborator

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.

Copy link
Copy Markdown
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 force-pushed the refactor-invites branch 2 times, most recently from 602431d to 31320bb Compare August 20, 2025 07:33
jonodrew requested a review from olleolleolle August 20, 2025 07:43
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 <3410350+jonodrew@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL