Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
993 bytes

Attached.

tim.plunkett’s picture

Uh, sorry. Forgot date was from core, so patch filed here: #1177220: Expand coverage of ctools_dependent_pre_render

Still, here's a patch for the various elements provided by the date modules.

KarenS’s picture

Status: Needs review » Postponed (maintainer needs more info)

I have no problem with this conceptually, just trying to understand what it will do. Can you explain what this will accomplish?

tim.plunkett’s picture

Status: Postponed (maintainer needs more info) » Needs review

Views still uses ctools's dependency system instead of #states. Within Views, to show another input only when a date is set to a given value, it needs to use this #pre_render callback.

KarenS’s picture

Status: Needs review » Reviewed & tested by the community

OK, looks fine to me.

arlinsandbulte’s picture

Status: Reviewed & tested by the community » Needs work

patch no longer applies

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
4.27 KB

I'm surprised git wasn't smart enough to figure that out.
Here's a reroll.

arlinsandbulte’s picture

Status: Needs review » Needs work

Sorry, needs another reroll....

arlinsandbulte’s picture

Also, I noticed a bunch of code style changes like this:

-    );
+  );

That only change the indentation of that bracket.
I am not very familiar with coding standards, but is that intentional?

tim.plunkett’s picture

I normally don't try to fix coding standards in patches, but since I was modifying the code directly in that function, I figured it was reasonable. See http://drupal.org/coding-standards#array, the changes are valid.

However, in the interest of getting something committed, here are both patches.

arlinsandbulte’s picture

Status: Needs review » Fixed

OK, thanks for the explanation. I'm ok with correcting the style at the same time. Just wanted to make sure and learn in the process.

Committed date-1177198-10.patch from #10.
http://drupalcode.org/project/date.git/commit/67cbfaf

Note:
I did not use git am because I wanted to update the changelog on the same commit.
Instead, I used git apply and then git commit --author='tim.plunkett'
Perhaps there is a way to use git am, edit the changelog, then modify the commit to include the changelog edit... all of this done locally, of course, and then pushed.
Is that possible?

tim.plunkett’s picture

@arlinsandbulte, After git am, make more changes, then git commit --amend will add those into the previous commit, then you can push.

EDIT: You should also research "git interactive rebase", git rebase -i, it's how you would combine unpushed commits that aren't sequential, among other things, and it is amazing. If you ever catch me on IRC, I'd be happy to walk you through it, or people in #drupal-gitsupport could explain it, or you can read about it online.

Status: Fixed » Closed (fixed)

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

MustangGB’s picture

Issue summary: View changes
Status: Closed (fixed) » Needs review
FileSize
769 bytes

Follow-up to allow #dependency on filters.

MustangGB’s picture

DamienMcKenna’s picture

Status: Needs review » Reviewed & tested by the community

I think this is ok.

The last submitted patch, 1: date-1177198-1.patch, failed testing.

The last submitted patch, 2: date-1177198-2.patch, failed testing.

The last submitted patch, 7: date-1177198-7.patch, failed testing.

The last submitted patch, 10: date-1177198-10.patch, failed testing.

The last submitted patch, 10: date-1177198-10-NOCODINGSTANDARDSFIXES.patch, failed testing.

DamienMcKenna’s picture

Rerolling.

DamienMcKenna’s picture

DamienMcKenna’s picture

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks all.

MustangGB’s picture

Much appreciated.

Status: Fixed » Closed (fixed)

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