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

Add about section #86

Closed
wants to merge 4 commits into from
Closed

Conversation

kloimhardt
Copy link

This pull request addresses issue #63. It is meant to start things going, as there are some open points:

  • what should be written in the "About" text, especially
  • how do I add the external link to clojurians.net (do I use [:a {:href ...] correctly)?
  • which css classes should be used for formatting, especially
  • where is the "About..." link to be put on the main page

@codecov-io
Copy link

codecov-io commented Mar 12, 2020

Codecov Report

Merging #86 into master will decrease coverage by 1.61%.
The diff coverage is 14.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #86      +/-   ##
==========================================
- Coverage   46.53%   44.92%   -1.62%     
==========================================
  Files          21       21              
  Lines        1053     1113      +60     
  Branches       49       49              
==========================================
+ Hits          490      500      +10     
- Misses        514      564      +50     
  Partials       49       49              
Flag Coverage Δ
#unit 44.92% <14.92%> (-1.62%) ⬇️
Impacted Files Coverage Δ
src/clojurians_log/db/queries.clj 56.96% <0.00%> (-2.25%) ⬇️
src/clojurians_log/views.clj 15.65% <8.88%> (-1.33%) ⬇️
src/clojurians_log/routes.clj 26.12% <33.33%> (+1.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d14381...a11fdfa. Read the comment docs.

@plexus
Copy link
Member

plexus commented Mar 13, 2020

Hey, this is a great start, nice 💚 ! Some more things you can mention in the about text

  • the source code is in this github repo, you are welcome to contribute
  • if a channel is not logging it's probably because @logbot isn't receiving its messages. invite @logbot to a channel to start logging
  • Exoscale is kindly donating hosting

You'll have to figure out where is a good place to link to the about page. Maybe in the footer, or maybe we can add a header. This would be a good opportunity to have a better look at some of the CSS.

bump_it

@kloimhardt
Copy link
Author

you mentioned in a comment, that at some point you'd prefer to delete legacy.css and style.css and do the styling over in clean Garden+Tachyons.
Do you have any example (site or tutorial or repo) where I could get an idea how to achieve this? The first step would be to reproduce exactly what we have now with this "clean Garden+Tachyons" (where I do not yet have a clear idea what that exactly means in terms of concrete coding task).

Comment on lines +85 to +88
(let [{:keys [chan-day-cnt chan-name->id] :as index}
(if (seq @!indexes)
@!indexes
{:chan-day-cnt {} :chan-name->id {}})]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a hack to solve #61. I'd like to find a clean solution. But the suggested (defonce !indexes (atom {:chan-name->id {} ,,,})) does not work, because after calling (go) at the repl, !indexes is reset to empty map {}. Question: which code is actually called when I type (go) at the repl?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!indexes is already dereferenced on line 84, so instead of @!indexes just use indexes. I should have done that too on line 85. But this doesn't seem like the right solution, the when-let means that this function may return nil if there are no indexes, so the consumer should be able to work with that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(go) calls (user/go), which calls (reloaded.repl/go). This initializes the system (see user.clj:15) and starts it, so it starts all the components you see clojurians-log.application/prod-system + garden-watcher.

I would approach this differently, and make sure the indexes are nil when they are not built yet, or are built but there is no data. so change clojurians-log.db.queries so that (def !indexes (atom nil)), and on line 26 in the reduce replace the empty map with nil. This is fine because (assoc-in nil [...] ...) will return a map. This means channel-days will just work, either it returns a value, or nil. Then it's a matter of making sure the views work with that. Having a quick look I think we need an (if channel-days ...) branch in log-page-header

:clojurians-log.routes/channel
{:channel name})}
"# " name]])]
[:p "no data loaded"])]]])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for #61. Question: is there a more suitable text than "no data loaded", especially for the production environment?

Comment on lines +40 to +46
(comment
(go)
(reset)
(reset-all)
(use 'clojurians-log.repl)
(load-demo-data! "../clojurians-log-demo-data")
#_:end)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful snippets for repl-driven development in Sapcemacs. Question: is there a better place to put this? Or do you bind those to some keystrokes in your IDE? Or in general: which keystrokes do you bind to which commands for repl-driven development?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine to have it in a comment here. Alternatively you can put it in a file in the repl/ directory.

I also use Spacemacs, with standard bindings like , e e to eval. I also have some leader key bindings myself.

(defun plexus-clojure-extras/cider-pprint-register (register)
  (interactive (list (register-read-with-preview "Eval register: ")))
  (cider--pprint-eval-form (get-register register)))

(dolist (m '(clojure-mode
               clojurec-mode
               clojurescript-mode
               clojurex-mode
               cider-repl-mode
               cider-clojure-interaction-mode))

    (spacemacs/set-leader-keys-for-major-mode m
      "ep" 'cider-pprint-eval-last-sexp
      "eP" 'cider-pprint-eval-last-sexp-to-comment
      "en" 'cider-eval-ns-form
      "er" 'cider-eval-last-sexp-and-replace
      ","  'plexus-clojure-extras/cider-pprint-register
      "lp" 'sesman-link-with-project
      "lb" 'sesman-link-with-buffer
      "lb" 'sesman-link-with-directory
      "ll" 'sesman-link-with-least-specific
      "ss" (if (eq m 'cider-repl-mode)
               'cider-switch-to-last-clojure-buffer
             'cider-switch-to-repl-buffer)
      "'"  'cider-jack-in-clj
      "\"" 'cider-jack-in-cljs
      "\&" 'cider-jack-in-clj&cljs
      "sq" 'cider-quit
      ))

I use , e p a lot. , e P will insert the result in the buffer in a comment, or you can use SPC u , e e to insert directly.

The bottom ones are to undo the recent keybinding changes in the spacemacs clojure layer.

I also have a special , , <register> binding, so I can put snippets in a register and eval them from anywhere. I don't use it a lot, but sometimes it's very handy.

Comment on lines +276 to +283
[:ul
(for [[{:channel/keys [name]} channel-days] channel-day-tuples]
(for [[day cnt] channel-days]
[:li [:a {:href (path-for context
:clojurians-log.routes/channel-date
{:channel name
:date day})}
"# " name " " day " (" cnt ")"]]))]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempt to solve #50

@plexus
Copy link
Member

plexus commented Mar 15, 2020

Hey @kloimhardt, nice work! 🤩 Seems you are trying to tackle multiple issues at once here. Would you mind splitting that up into separate pull requests, one for each issue? It is not clear to me which of these three issues you're tackling is ready to be merged, and which are still in progress. It's also harder to evaluate and discuss them when they are mixed together like this.

you mentioned in a comment, that at some point you'd prefer to delete legacy.css and style.css and do the styling over in clean Garden+Tachyons

That was really just a suggestion reflecting my own preferences at the time. legacy.css is what we inherited from when there was no real app, just a bunch of node scripts to generate HTML. It looks like it was generated with SCSS, but I no longer have those sources. style.css is the bit of styling we've done on top of that since. So we're in a situation where the CSS we have is not a lot of fun to maintain, and where we definitely have room for improvement. For instance, Google penalizes us on mobile because links are too small and too close together. In general we don't have a proper mobile view. Some responsiveness would be great.

What to do with that is largely up to whoever decides to do something about that, what their level of CSS and personal preferences are, and how much time they want to put in. We could make incremental improvements upon what we have, or start from scratch, in plain CSS, or with some framework, or using Garden... I personally still like Garden, but people who really do a lot of frontend seem to prefer things like plain CSS or SCSS. I think Tachyons is great if you're good at it as it allows very rapid prototyping. (I don't use it enough to properly remember the classes, and then it's less effective.) Recently I've used Tailwind CSS on a project, which seems to be a better Tachyons.

So it's really up to you. This project is what people make of it. I'm just the guardian providing some guidance and historical background 😄

@kloimhardt
Copy link
Author

Thank you for all the valuable information.

After doing some research on SCSS/Tachyons/Tailwind, I will stick to Garden because it seems the go-to option for a Clojure project when not having some already formed CSS preference :-).

I will leave this master->master pull request as it is and start with very small ones again. Here, I was more following point 4 of the Clojure Etiquette "I am trying to make something and am having trouble, please help" and not point 1 "I made something - here is my contribution". Sorry for creating the confusion. Eventually we should get rid of this master->master pull request without merging anything from it.

@plexus
Copy link
Member

plexus commented Mar 17, 2020

Alright! looking forward to the individual PRs! Let me know if you like any more help or input from me.

@plexus plexus closed this Apr 1, 2020
@plexus
Copy link
Member

plexus commented Apr 1, 2020

Closing this as we are not planning to merge it. The things we want to keep can be brought up in separate PRs.

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