Skip to content

Conversation

@BryanQuigley
Copy link
Collaborator

First one definitely makes sense. (uses less space, assuming no bugs)

Second bit - dropping Gemfiles out of root and using Debian packages by default might have other consequences

@timwis
Copy link
Owner

timwis commented Sep 12, 2025

Hey Bryan, hope you're well. I agree the first commit is pretty clear cut. Can't quite understand the motivation behind the second one though. Haven't seen this before. Wouldn't that likely mean the docker version is behind the gemfile version?

@BryanQuigley
Copy link
Collaborator Author

Hi Tim, ty, hope you are well too.

The Gemfile is already set at the same major version of jekyll as the Debian packages. I'm just thinking one less thing to update on a regular basis for security updates - but it means more work to keep Debian packages/Gemfiles in sync every time we bump the Debian release.

Open to dropping if the trade off doesn't seem worth it.

@BryanQuigley
Copy link
Collaborator Author

Happy to proceed with the change or drop it. Even a vague preference is enough for me. Ty!

@timwis
Copy link
Owner

timwis commented Oct 19, 2025

Hey Bryan, apologies for not coming back on this sooner! Regarding the second commit, in my opinion, the risk of drift between the Gemfile version and the debian version outweighs the maintenance benefit. Debian packages are known for their stability, but partly because they're very slow to upgrade. I appreciate they're at the same minor version today, but that's only because we're behind schedule on upgrading to the latest minor version ourselves. For example, if we wanted to upgrade JKAN to use jekyll 4.4.1, there's no way to do that with the debian repository, so we'd either have to wait for debian to upgrade upstream or undo this change.

I think another way to solve the underlying problem—if I'm understanding your correctly—is to make better use of dependabot PR upgrades. What do you think?

Also, very happy to approve the first commit—and thanks for doing it!

@BryanQuigley BryanQuigley mentioned this pull request Oct 20, 2025
@BryanQuigley
Copy link
Collaborator Author

Closing per discussion.

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.

3 participants