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

Refactor rendering logic into separate methods #1158

Open
wants to merge 1 commit into
base: open-repo-type-state
Choose a base branch
from

Conversation

jrray
Copy link
Collaborator

@jrray jrray commented Dec 5, 2024

This change was prompted by wanting to refactor the code to work towards
having different trait bounds for the functions that need a repository
that supports renders (e.g., rendering into a repository) versus
functions that render into a plain directory.

Rework RenderType so the method that renders into a hardlink doesn't
need to worry about getting a RenderType::Copy value.

@jrray jrray added the pr-chain This PR doesn't target the main branch, don't merge! label Dec 5, 2024
@jrray jrray self-assigned this Dec 5, 2024
@jrray
Copy link
Collaborator Author

jrray commented Dec 5, 2024

@rydrman do you have a use case for hard linking into external directories?

@rydrman
Copy link
Collaborator

rydrman commented Dec 5, 2024

Yes, we leverage this quite extensively. Our storage is read-only for users and the permissions are standardized so we hardlink nearly all of our large deployments

@jrray
Copy link
Collaborator Author

jrray commented Dec 5, 2024

Okay I guess I need to rethink my refactoring. However don't you agree that this is a dangerous feature to support, in general? I think it might be good to gate it behind some kind of opt-in setting or something, so people understand the consequences.

@jrray jrray force-pushed the no-hard-links-for-dir-renders branch from 1ad0078 to 7ec8e8b Compare December 5, 2024 22:48
@jrray jrray changed the title Disallow hard link rendering to external directories Refactor rendering logic into separate methods Dec 5, 2024
@jrray jrray force-pushed the open-repo-type-state branch from 086c6b9 to d08ddba Compare December 10, 2024 18:26
This change was prompted by wanting to refactor the code to work towards
having different trait bounds for the functions that need a repository
that supports renders (e.g., rendering into a repository) versus
functions that render into a plain directory.

Rework `RenderType` so the method that renders into a hardlink doesn't
need to worry about getting a `RenderType::Copy` value.

Signed-off-by: J Robert Ray <[email protected]>
@jrray jrray force-pushed the no-hard-links-for-dir-renders branch from 7ec8e8b to 51a8e45 Compare December 10, 2024 18:58
@rydrman
Copy link
Collaborator

rydrman commented Dec 11, 2024

A warning or extra confirmation on the command line maybe since that's really the only place you can invoke it like that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-chain This PR doesn't target the main branch, don't merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants