* Zajcev Evgeny <zevlg@xxxxxxxxx> writes:
> Steve Youngs <steve@xxxxxxxx> writes:
>> * Zajcev Evgeny <lg@xxxxxxxx> writes:
>> > * lisp/xwem-weather.el
>> _*WAIT*_!! Don't do anything drastic like commit a --version-0
>> on this yet. I have a some issues with these changes and I need
>> some time to go over the code so the forthcoming arguments and
>> fights we're gonna have over it will be more fulfilling. :-P
> Haha, sure man, waiting for your comments ..
OK, here they come.
,----
| (eval-when-compile
| (autoload 'url-retrieve "url"))
`----
I _strongly_ object to this. The reason I wrote xwem-weather the way
I did was so that it could do the fetching _without_ using any
external (to XWEM/XLIB) libraries.
IMO this adds a dependency to XWEM that really is not worth the
bother. Now, I know you are going to come back with "using url
package means we can go through a proxy if needed"... true... BUT,
for that to work, the luser must have W3 configured to do so.
Would you like to take a guess at how big a number the subset of
users who _must_ access the web via a proxy, _and_ use W3, _and_
have W3 proxies configured right, _and_ use XWEM, _and DO NOT_
have a transparent proxy where their firewall automatically routes
all outgoing port 80 traffic through the proxy anyway? I'd bet my
left one that the answer was lower than one.
And even if there were a 1000 such users, I'd rather see xwem-weather
handle proxy connections by itself instead of dragging in code that
hasn't been updated since before you were born.
,----
| (defun xwem-weather-data-file ()
| "Return filename where to store/read weather info."
| (expand-file-name (concat "weather-" xwem-weather-station-id)
| xwem-weather-data-directory))
`----
This isn't a biggie, and I really don't care either way, but why
change from...
~/.xwem/ybbn
to...
~/.xwem/weather-ybbn (using Brisbane, AU ID as an example)
Seems to me that the `(concat "weather-"' is just fluff. Good idea to
change it to a defun though. :-)
,----
| ;;;###autoload
| (defun xwem-weather (&optional dockid dockgroup dockalign)
| "Initialise the weather dock."
| (interactive)
|
| (when (xwem-osd-p xwem-weather-osd)
| (xwem-osd-destroy xwem-weather-osd))
| (when (itimerp (get-itimer "xwem-weather-itimer"))
| (delete-itimer (get-itimer "xwem-weather-itimer")))
| (xwem-weather-display-temp 'force)
| (unless (zerop xwem-weather-frequency)
| (start-itimer "xwem-weather-itimer"
| 'xwem-weather-update
| xwem-weather-frequency
| xwem-weather-frequency)))
|
| (defalias 'xwem-weather-init 'xwem-weather)
`----
What's the point of renaming `xwem-weather-init' to `xwem-weather' and
making it interactive? This really isn't something a user needs to
get confused over. If you really do want something interactive, how
about...
(defun xwem-weather-brute-force-reload ()
"Kill your weather dock and reload a new one in a totally different
spot on your systray. Normal people would never do this, but I guess
you know what you are doing, so have at it, buddy."
(interactive)
(xwem-weather-init))
> Actually i was planning to commit version-0, but was too drunk to do
> that .. i simple forgot to specify --seal flag :)
Dude, you're nuts! That last changeset of yours had quite a lot of
shit in it. Some of it no doubt will be a bit destabilising. On a
changeset like that you need to let it settle for a while before you
seal it.
>> > * dockapp/xwem-mpd.el:
>>
>> Any reason why you didn't merge in my --patch-19? You know, that
>> merge req I sent on 7th December... _LAST YEAR_!!
> Hmm, yeah only one reason - i missed it :) i will take a look at it,
Thanks.
--
|---<Steve Youngs>---------------<GnuPG KeyID: A94B3003>---|
| In space, |
| No one can hear you rip a stinky |
|---------------------------------------<steve@xxxxxxxx>---|
pgpw9QTZS7EQj.pgp
Description: PGP signature
|