solarpunk-guile
/
guile-termenv
3
0
Fork 1
Code Issues Pull Requests Releases 1 Activity

Turn repo into a guix channel #4

Merged
juliana merged 7 commits from :main into main 2024-03-01 21:21:47 +00:00
Conversation 2 Commits 7 Files Changed 4 +97 -75
TakeV commented 2024-02-27 01:37:23 +00:00
Owner
Copy Link

With this, the repo itself is a guix channel, thus allowing anyone to use it as a package source.

With this, the repo itself is a guix channel, thus allowing anyone to use it as a package source.
TakeV added 1 commit 2024-02-27 01:37:25 +00:00
vv requested review from juliana 2024-02-27 12:40:34 +00:00
vv commented 2024-02-27 12:41:05 +00:00
Owner
Copy Link

i'll let juli review as i dont really get the details here.

i'll let juli review as i dont really get the details here.
TakeV added 1 commit 2024-02-28 00:34:25 +00:00
TakeV commented 2024-02-28 00:35:11 +00:00
Author
Owner
Copy Link

Fixed the merge conflict.

Fixed the merge conflict.
TakeV added 2 commits 2024-02-28 00:45:41 +00:00
juliana reviewed 2024-02-29 21:09:04 +00:00
juliana left a comment
Owner
Copy Link

I've provided a few comments and opened some discussion about how things are done. As always, feel free to disagree with or straight up ignore my comments :3

In general, as long as the channel works, this should be fine. Just make sure the channel works.

ETA: I forgot to mention, the channel should also be documented in both the README and the .texi file.

I've provided a few comments and opened some discussion about how things are done. As always, feel free to disagree with or straight up ignore my comments :3 In general, as long as the channel works, this should be fine. Just make sure the channel works. ETA: I forgot to mention, the channel should also be documented in both the README and the `.texi` file.
.guix/modules/guile-termenv.scm Outdated
@ -0,0 +1,49 @@
(use-modules (gnu packages)
juliana commented 2024-02-29 20:53:52 +00:00
Owner
Copy Link

Doesn't this need to be a define-module to work properly, with the package as a publicly-exported symbol?

Doesn't this need to be a `define-module` to work properly, with the package as a publicly-exported symbol?
TakeV commented 2024-03-01 20:33:34 +00:00
Author
Owner
Copy Link

Oh yeah, it does. Lemme fix that.

Oh yeah, it does. Lemme fix that.
TakeV marked this conversation as resolved
guix.scm Outdated
juliana commented 2024-02-29 21:06:45 +00:00
Owner
Copy Link

I don't think this is quite the right move. IIUC the package object needs to be in a Guile module and exported publicly so it can be imported. Meanwhile, the guix.scm file needs to evaluate directly to a package object. I think the optimal route here is to define the package in the module file then load and import that file into guix.scm, which would then simply return the symbol for the package.

So guix.scm would be something like...

(load ".guix/modules/guile-termenv.scm")
(use-modules (guile-termenv))

guile-termenv

I'm also not sure about the naming scheme for the package file and module. guile-termenv makes sense for the Guix name for the package, and as a general name for the project to differentiate from the original; but it looks weird in a module name. Maybe instead make the namespace termenv package or termenv-package? The former would give us a file called .guix/modules/termenv/package.scm while the latter would give us .guix/modules/termenv-package.scm. I dislike the hyphenation in a module name, but it's probably more reasonable than adding a mostly-empty directory.

I don't think this is quite the right move. IIUC the package object needs to be in a Guile module and exported publicly so it can be imported. Meanwhile, the `guix.scm` file needs to evaluate directly to a package object. I think the optimal route here is to define the package in the module file then `load` and `import` that file into `guix.scm`, which would then simply return the symbol for the package. So `guix.scm` would be something like... ``` (load ".guix/modules/guile-termenv.scm") (use-modules (guile-termenv)) guile-termenv ``` I'm also not sure about the naming scheme for the package file and module. `guile-termenv` makes sense for the Guix name for the package, and as a general name for the project to differentiate from the original; but it looks weird in a module name. Maybe instead make the namespace `termenv package` or `termenv-package`? The former would give us a file called `.guix/modules/termenv/package.scm` while the latter would give us `.guix/modules/termenv-package.scm`. I dislike the hyphenation in a module name, but it's probably more reasonable than adding a mostly-empty directory.
TakeV commented 2024-03-01 21:06:14 +00:00
Author
Owner
Copy Link

I went ahead and switched the format to be the load/import/return style you suggested.

I went ahead and switched the format to be the load/import/return style you suggested.
juliana marked this conversation as resolved
hall.scm Outdated
@ -17,4 +19,3 @@
(native-language-support #f)
(licensing #f)))
(files (libraries
((directory "termenv"
juliana commented 2024-02-29 20:56:10 +00:00
Owner
Copy Link

This indentation is all kinds of wonky; we need to setup a .dir-locals.el for this project so we can get consistent styling. I'll try to remember to cannibalize the Guix one after this review; it's not all useful outside of Guix proper, but it's a good starting point.

This indentation is all kinds of wonky; we need to setup a `.dir-locals.el` for this project so we can get consistent styling. I'll try to remember to cannibalize the Guix one after this review; it's not all useful outside of Guix proper, but it's a good starting point.
TakeV commented 2024-03-01 20:58:20 +00:00
Author
Owner
Copy Link

Regrettably, I think the formatting is due to hall itself. :(

Regrettably, I think the formatting is due to hall itself. :(
juliana marked this conversation as resolved
TakeV added 2 commits 2024-03-01 21:06:01 +00:00
f1d09c368d
Make the guix.scm file load guile-termenv and return it 
This means we no longer use a symlink. We also needed to define a
module for the guile-termenv file.
juliana requested changes 2024-03-01 21:12:52 +00:00
juliana left a comment
Owner
Copy Link

Aside from that one small comment, LGTM. Make that change and it'll be ready to go!

Aside from that one small comment, LGTM. Make that change and it'll be ready to go!
.guix/modules/termenv-package.scm Outdated
@ -0,0 +1,50 @@
(define-module (termenv-package))
(use-modules (gnu packages)
juliana commented 2024-03-01 21:09:33 +00:00
Owner
Copy Link

Make sure to put these in the module definition

Make sure to put these in the module definition
TakeV commented 2024-03-01 21:20:38 +00:00
Author
Owner
Copy Link

Fixed.

Fixed.
juliana marked this conversation as resolved
TakeV added 1 commit 2024-03-01 21:20:29 +00:00
juliana merged commit d02e256bb7 into main 2024-03-01 21:21:47 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
juliana
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: solarpunk-guile/guile-termenv#4
No description provided.
Delete Branch ":main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?