-
Notifications
You must be signed in to change notification settings - Fork 83
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
Added config variations #82
base: master
Are you sure you want to change the base?
Conversation
Hi, first kudos for writing documentation on new features! I amenable to this change with one variation. Instead of a variation name we should pass the pathname to the different configuration file. This would simplify the implementation to something along the lines of. (defun load-config (&optional (repo-dir "") config-file)
(with-open-file
(in (or config-file
(discover-config-path repo-dir))
:external-format :utf-8)
...)) Minor nitpicks:
Anyhow my 2c, @redline6561 What do you think? |
Definite kudos for documentation updates with new features. 🍰 🎉 The one thing I wonder is if this could be incorporated in the Coleslaw CLI / command line app work that you were doing @PuercoPop. Might be a more natural fit as just a parameter to a "build" tool. Granted that CLI work might not be ready for merge, not sure of status. 👍 on those nitpicks. I love the "Flavors" shoutout, did you buy one of those LispMs from the eBay auctions? ;) On a more general/personal note, I've finally gotten a handle on my new job as a programming instructor here in the second semester. Am looking forward to being more active on the Coleslaw front in 2-3 weeks. Top task on my list is getting us a real test suite, maybe fiveam, maybe stefil, maybe fiasco. Then helping get the other open pull requests and stuff from forks merged in. 🍪 |
If I understood right, stefil is decrecated and fiasco is its sucessor. |
@arademaker not quite. stefil is deprecated in favor of hu.dwim.stefil. Fiasco is a fork from stefil fork. Their differentiating feature is providing restarts for interactive debugging when a test fails. As of late I've favored prove as it doesn't require any ceremony (declaring a suite or a fixture or a test package) in order to begin writing and running tests. @redline6561 I'd wish :'(. |
Its no clear for me what i have to change to merge this. I can edit this for the next week because now i have some exams. As @PuercoPop said, is better to use the path to a configuration file, and i think that nothing more than that, because in the configuration can be the repo dir. I think that this could be better than "variations" or "flavors". An then we will have only: (defun load-config (config-file) ... ) A question.... do i have to close this commit? (: |
@DiGiorgi sorry for the late reply. git checkout <last-commit-from-upstream> -b patch-2
*new commits go here changes*
git push -f <fork>* patch-2:master *this is the name of the remote of your github repo of coleslaw. if you need any help, you can hit me up on #lisp. Also no rush so it can be after your exams. |
- main takes an additional optional parameter to specify the configuration file Related Issue #82
Coleslaw needs a `.coleslawrc` file to operate properly. That file is usually located at | ||
$HOME/.coleslawrc but may also be placed in the blog repo itself. | ||
Coleslaw needs a configuration file to operate properly. It can has any name and | ||
is usally placed in the blog repo itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value should be mentioned/kept though.
Also typos, "usually" and "have" instead of "has".
I'm all for being able to choose a different configuration file. Can we perhaps allow users to use the |
looking into this. |
Any update? |
This allow that the user con choose different config files for the same project. You can see it explained in the documentation i changed in docs/config.md.