In the 7.x port of this module I introduced a few clean-ups because some things weren't working the way they should have and some parts of the code are really ugly in 6.x now.
Also #742020: Create option to use users Local time will be introduced into 6.x first.

Anyways, the title of the issue explains what needs to be done!

Also, #706594: Integrate with Libraries API will introduce a temporary code-clutter due to handling of mutliple possible library locations, but that will eventually vanish in the 6.x version. I will not introduce that code in the 7.x version (it is merely smoothing the update path of people, nothing more) until Drupal 7.0 is released. If we are lucky we can get #706594: Integrate with Libraries API all the way through before that, So Clock-7.x-1.0 will be fully Libraries API compliant. We'll see.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

Clock module also baldy needs some help fix-up.
Currently there is no hook_help and no README.txt.
Also some (actually useful) inline help on the block admin screen would be nice.
Maybe do this as part of this patch? Or in a seperate issue?

tstoeckler’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Assigned: Unassigned » tstoeckler

Will work on this in the next few days.
Created #755402: Create help for the help issue. That will probably go in first.

A few details on what I want cleaned up:
- Instead of declaring the whole form twice in the if{}, just re-declare what is necessary (already in D7).
- Modularize the huge theme_clock() into helper functions to get the current time zone, etc. Because such a function could also be used for the formatting of the date format options (where we show the time (in the current time zone) formatted in the specific format). (already in D7, might be cleaned up there as well).
- Stop namespacing of the variables (PHP and JS). That's just silly. I'll see if it does make sense anywhere (eg to avoid confusion between clock and date api parameters) but definitely remove most of that, hopefully all.
- In d7: The #states stuff in the form didn't work, last time I tried. Work that out.
- Provide properly named keys for the JS settings.

And anything that bugs me when I look at the code in detail. :)

tstoeckler’s picture

Another one on the list (could be a separate issue though):
- in d7: Clock doesn't work well with the overlay. When you click the close button, the clock is all jittery. Reloading takes care of that, but still. Could be a core bug. Don't know.

tstoeckler’s picture

Also:
- Update SimpleTests for #742020: Create option to use users Local time.
- Update translation template.

tstoeckler’s picture

Also:
- Update CHANGELOG.txt

tstoeckler’s picture

Also:
- Improve INSTALL.txt

tstoeckler’s picture

FileSize
19.09 KB

Cleanup of clock.info, clock.install and clock.module.Also renames INSTALL.txt to README.txt and updates that for the new location of jqueryTimers. That way we can provide additional information on how the module works, etc. in the future but don't have to put that in an isolated file.

Polishing D6 first and then syncing with D7.

Still to do:

  • SimpleTests
  • CHANGELOG.txt
  • translation template
  • clock.js clean-up

Concerning the last one: I recently noticed that newer versions of PHP have some new date formatters (http://php.net/date). Therefore clock.js will need a major overhaul.

tstoeckler’s picture

And the commit link.
http://drupal.org/cvs?commit=353126

"Back to active."

tstoeckler’s picture

Status: Active » Needs review
FileSize
1.36 KB

I seemed to have messed up something with the different files in the branches, so that needs to be revisited before release.

I am using this post as a test of the newly enabled Testing Framework for Clock, which I noticed in another issue recently.

Status: Needs review » Needs work

The last submitted patch, clean-up.patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review
FileSize
1.35 KB

Let's try this again. I can't really believe that there were windows line endings before, but hey.

tstoeckler’s picture

FileSize
862 bytes

So I guess it won't test Drupal 6.
So I attached a small (!!!) cleanup patch for D7.

tstoeckler’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
tstoeckler’s picture

FileSize
861 bytes

Windows line-endings again? I'm on Ubuntu! WTF?
Oh well, let's see if this one works.

Status: Needs review » Needs work

The last submitted patch, cleanup7.patch, failed testing.

tstoeckler’s picture

FileSize
2 KB

CHANGELOG.txt

Committed this so leaving at needs work for the rest.

tstoeckler’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs work » Needs review
FileSize
36.97 KB

This is the clean-up for the JS-file. It makes the formatting function independent of all the clock-formatting, which is how it should be.
Not committing yet, because it is untested.

tstoeckler’s picture

FileSize
37.37 KB

Now with the updates up to PHP 5.3 at http://php.net/date

Will tackle the .test file now, then test it, then commit.

That will be the last release blocker.
Will sync D7 after that.

tstoeckler’s picture

FileSize
41.4 KB

Now that should be it.

tstoeckler’s picture

FileSize
40.01 KB

Wow, the last one didn't even remotely work. Good thing, I didn't commit that one.
This one works, though, so I am commiting that one.

tstoeckler’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
FileSize
45.81 KB

This makes the D7 .js file and .module files as nice-looking as the D6 counterparts.

Will tackle the .test file now and then commit.

tstoeckler’s picture

FileSize
7.65 KB

Here's one for the tests. It's untested, but it was completely broken before, so I'm just hoping I didn't introduce any syntax errors.

tstoeckler’s picture

Status: Needs review » Fixed

Since Simpletesting is currently postponed on a bug in Date API, I decided to commit this.
http://drupal.org/cvs?commit=362604

Marking fixed!
Will hopefully run the tests manually in the next few days.

tstoeckler’s picture

FileSize
10.65 KB

Last one itroduced some errors. The JavaScript would not load, there was a notice on every page and block admin would result in a fatal.
Not nice...
Leaving at fixed, because I'll commit this right away.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.