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

feat:mkYarnPackage template #43

Merged
merged 3 commits into from
Sep 13, 2022
Merged

feat:mkYarnPackage template #43

merged 3 commits into from
Sep 13, 2022

Conversation

samrose
Copy link
Contributor

@samrose samrose commented Sep 7, 2022

No description provided.

@samrose samrose requested a review from mkenigs September 7, 2022 17:16
@limeytexan
Copy link
Contributor

We know from other languages that there can be multiple builders over time, often for the very same underlying ecosystem (e.g. buildRustPackage vs naersk.buildRustPackage). In that respect the path templates/javascript-yarn/flake.nix may not be a good choice. A better choice might be the name of the builder itself, because that exists in the Nix namespace and is guaranteed to be unique.

Another question - do we want to inject a directory level within templates for builders specifically? What other kinds of templates might we want to use, or are builders the only use for flake templates?

These are also questions that can answered later with a treewide refactoring so there's no need for this to block progress now, but thought I'd put it out there before we get too far down this path.

Thanks for building out the library of templates for flox init!!! :-)

@mkenigs
Copy link
Contributor

mkenigs commented Sep 7, 2022

A better choice might be the name of the builder itself, because that exists in the Nix namespace and is guaranteed to be unique.

If we do that, I think currently the folder name will be displayed in the flox init menu. Not sure if that's good or bad, just making a note

Another question - do we want to inject a directory level within templates for builders specifically? What other kinds of templates might we want to use, or are builders the only use for flake templates?

I think that could be nice, but right now capacitor doesn't support that

@limeytexan
Copy link
Contributor

If we do that, I think currently the folder name will be displayed in the flox init menu.

Currently, sure - but the flox cli sits in a position where it can massage the data returned by nix and turn that into a hierarchical menu driven experience. As long as we adhere to a convention we can make it look right to the user.

Overall theme as with so many things is to think outside the Nix box.

@mkenigs
Copy link
Contributor

mkenigs commented Sep 9, 2022

Currently, sure - but the flox cli sits in a position where it can massage the data returned by nix and turn that into a hierarchical menu driven experience. As long as we adhere to a convention we can make it look right to the user.

Agreed, just flagging as a work item!

@samrose
Copy link
Contributor Author

samrose commented Sep 13, 2022

A better choice might be the name of the builder itself, because that exists in the Nix namespace and is guaranteed to be unique.

If we do that, I think currently the folder name will be displayed in the flox init menu. Not sure if that's good or bad, just making a note

Another question - do we want to inject a directory level within templates for builders specifically? What other kinds of templates might we want to use, or are builders the only use for flake templates?

I think that could be nice, but right now capacitor doesn't support that

Per @limeytexan request, I submitted a PR to rename the existing templates to their builder at #50 I am going to follow up now on this PR to rename the folder to mkYarnPackage

@samrose samrose changed the title feat: javascript-yarn template feat:mkYarnPackage template Sep 13, 2022
@samrose samrose merged commit 57ce22d into master Sep 13, 2022
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