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.

Sign up for GitHub

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

Jump to bottom

Refactor(DRY): ./data/${name} #200

Closed
srid opened this issue May 26, 2024 · 6 comments · Fixed by #261
Closed

Refactor(DRY): ./data/${name} #200

srid opened this issue May 26, 2024 · 6 comments · Fixed by #261
Labels
good first issue Good for newcomers

Comments

@srid
Copy link
Member

srid commented May 26, 2024

Easy to break this convention in new PRs

image

e.g.: #195 (comment)
@shivaraj-bh
Copy link
Member

Shouldn't #53 prevent this?

@srid
Copy link
Member Author

srid commented May 26, 2024

Only if contributors always use that template and promise to never hand edit the generated dataDir option.

shivaraj-bh added a commit that referenced this issue Jun 13, 2024
@shivaraj-bh
fix(ollama): Broken dataDir convention; Allow ENVs in dataDir
f3a6146 
Easy to break the convention, see:
#200

double-quotting in shell script prevents globbing:
https://www.shellcheck.net/wiki/SC2086
@srid
Copy link
Member Author

srid commented Jun 13, 2024

Should we add namespace to this consideration as well?

c. #219 (comment)

@shivaraj-bh
Copy link
Member

shivaraj-bh commented Jun 13, 2024 •
edited
Loading

namespace and dataDir are going to follow the same convention across services, so how about creating a default module and importing it in https://github.com/juspay/services-flake/blob/main/nix/lib.nix?

Edit: To be specific, here:

services-flake/nix/lib.nix

Line 25 in 14a3740

modules = [ mod ];

@srid
Copy link
Member Author

srid commented Jun 13, 2024 •
edited
Loading

Good idea.

Make sure to use lib.mkDefault though.

shivaraj-bh added a commit that referenced this issue Jun 13, 2024
@shivaraj-bh
fix(ollama): Broken dataDir convention; Allow ENVs in dataDir
8f3d6f0 
Easy to break the convention, see:
#200

double-quotting in shell script prevents globbing:
https://www.shellcheck.net/wiki/SC2086
shivaraj-bh added a commit that referenced this issue Jun 13, 2024
@shivaraj-bh
fix(ollama): Broken dataDir convention; Allow ENVs in dataDir
db7ab71 
Easy to break the convention, see:
#200

double-quotting in shell script prevents globbing:
https://www.shellcheck.net/wiki/SC2086
@srid srid added the good first issue Good for newcomers label Jul 4, 2024
@srid
Copy link
Member Author

srid commented Jul 4, 2024 •
edited
Loading

This should be fairly simple to implement after #258 (which does it for namespace)

@shivaraj-bh shivaraj-bh mentioned this issue Jul 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: Do not repeat dataDir juspay/services-flake
2 participants
@srid @shivaraj-bh