coleslaw-org / coleslaw

Flexible Lisp Blogware
BSD 2-Clause "Simplified" License
553 stars 82 forks source link

Added config variations #82

Open herdigiorgi opened 9 years ago

herdigiorgi commented 9 years ago

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.

PuercoPop commented 9 years ago

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?

kingcons commented 9 years ago

Definite kudos for documentation updates with new features. :cake: :tada:

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.

:+1: 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. :cookie:

arademaker commented 9 years ago

If I understood right, stefil is decrecated and fiasco is its sucessor.

PuercoPop commented 9 years ago

@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 :'(.

herdigiorgi commented 9 years ago

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? (:

PuercoPop commented 9 years ago

@digiorgi sorry for the late reply. No need to close this pull request, just implement the amended changes in a new branch and push them to your PR branch, in this case master.

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.

Ferada commented 7 years ago

I'm all for being able to choose a different configuration file. Can we perhaps allow users to use the :repo-dir option directly in the configuration file as a default value such that only loading that file is sufficient to get all paths and options ready?

guicho271828 commented 4 years ago

looking into this.

dertuxmalwieder commented 3 years ago

looking into this.

Any update?