1

Resolved

Keeping it DRY: Cleanup declarations of FormValueRequiredAttribute

description

A number of modules redefine a very useful action method selector attribute: FormValueRequiredAttribute.
Another set of modules reuse the version from the Contents module in AdminController.
The usefulness of this attribute evident, I would like to propose we move this baby to Orchard.Framework/Mvc.

comments

Piedone wrote Jul 20, 2013 at 9:53 PM

Agreed, this has bugged me for a long time too!

sfmskywalker wrote Jul 20, 2013 at 10:52 PM

Excellent. I created a pull request here so we can incorporate as soon as the chief has agreed. :)

https://orchard.codeplex.com/SourceControl/network/forks/sfmskywalker/BulkFeatureManagement/contribution/5079

sfmskywalker wrote Jul 20, 2013 at 11:14 PM

sfmskywalker wrote Jul 21, 2013 at 12:04 AM

Fixed in changeset fcb0b921cb02

pszmyd wrote Jul 24, 2013 at 2:27 AM

This one is a major breaking change. Expect many existing modules to break - I just noticed Memcached and few of my own throwing exceptions...

pszmyd wrote Jul 24, 2013 at 2:28 AM

I meant "throwing build errors."

AimOrchard wrote Jul 24, 2013 at 10:19 AM

If you keep breaking things, you might as well call it V2.0 and give it its own, fresh gallery? :)

sfmskywalker wrote Jul 24, 2013 at 11:17 AM

Sorry for the inconvenience guys. But it is for the better. Being afraid to break things is a root cause for getting messy source code. In this case, a single class was duplicated 6 times! How can anyone sleep at night knowing this?

Piedone wrote Jul 24, 2013 at 11:50 AM

Image

AimOrchard wrote Jul 24, 2013 at 1:51 PM

I was just 'using' this work item to state my opinion: the gallery is already a mess, and this ain't helping, the gallery should get some lovin'

That said, I agree that if things might break for the greater good, you should break them, but you should also keep in mind what the result of the 'breaking' is, and how you handle it.

sfmskywalker wrote Jul 24, 2013 at 2:43 PM

@Zoltan: Admit it: you sleep better too. :)

sfmskywalker wrote Jul 24, 2013 at 2:45 PM

@AimOrchard: Agreed. Any suggestions on how to handle it? I am thinking maybe writing a quick post as a headsup for module project owners with instructions.

CSADNT wrote Jul 24, 2013 at 4:08 PM

The usual way is marking as [obsolete] for 1 release, then removing .... not the wheel :)

CSADNT wrote Jul 24, 2013 at 4:09 PM

This could be a way to differenciate obsolete modules: the ones compling with obsolete warnings....

sfmskywalker wrote Jul 24, 2013 at 4:47 PM

That's actually not a bad idea: we could move back the FormValueRequiredAttribute to the AdminController of the Contents module, marked as obsolete. Then kill it in 1.7.next. Thanks.

sebastienros wrote Jul 24, 2013 at 7:47 PM

Please

sfmskywalker wrote Jul 24, 2013 at 8:38 PM

Done. Don't kill me now.

sfmskywalker wrote Mar 28 at 12:28 AM

Fixed in changeset 42e2eb630ea471ac4cbb7a9dd95af68c1529f33c