You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by Jeff Tapper <je...@spoon.as> on 2012/03/26 17:41:37 UTC

minor Validator improvement

In my whiteboard, I just checked in a minor change to the validator class,
so the ValidateAll method will return a Vector of ValidationResultEvent
instances, rather than a generic array.    Since the array cant contain
anything other than ValidationResultEvent instances, a vector makes more
sense.  Additionally, the strong typing allows for code hinting and compile
time checking of the results.

 

If anyone has a few minutes, I'd appreciate a quick code review and any
feedback.

 

Another change im contemplating is for the argument to be a Vector of
IValidator instances, rather than a generic array.

 

Thoughts?


Re: minor Validator improvement

Posted by Omar Gonzalez <om...@gmail.com>.
>
>
> Adobe didn't care if we broke an app if it was for the "greater good".  And
> we didn't convert Arrays to Vectors everywhere.  So I don't think there can
> be a universal policy.  It has to be considered for each case because there
> are trade-offs.  If the array is passed across a MarshallPlan boundary,
> then
> I wouldn't change it.  If the array is likely to be passed back to the
> server, then I wouldn't change it as it might screw up AMF serialization.
> If the array is a vector of "anything" then I would be cautious.
>
>
I'm good with that, I think its a good change and the code hinting benefits
are good for developers.

-omar

Re: minor Validator improvement

Posted by Alex Harui <ah...@adobe.com>.


On 3/27/12 8:20 AM, "Jeff Tapper" <je...@spoon.as> wrote:

> Should I reopen it, as its not yet in the proper branch?  Im assuming
> changes like this may get moved to a branch after a vote on a release.
IMO, just leave it closed and put in a comment that says:  "Didn't need to
open a bug because I'm a committer and we are using commit-then-review".

-- 
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui


RE: minor Validator improvement

Posted by Jeff Tapper <je...@spoon.as>.
Should I reopen it, as its not yet in the proper branch?  Im assuming
changes like this may get moved to a branch after a vote on a release.

-----Original Message-----
From: Carol Frampton [mailto:cframpto@adobe.com] 
Sent: Tuesday, March 27, 2012 10:10 AM
To: flex-dev@incubator.apache.org
Subject: Re: minor Validator improvement



On 3/26/12 7 :07PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>> It is for me.  It is supposed to be commit-then-review, not
>> 
>>commit-then-open-jira-issue-then-resolve-jira-issue-then-close-jira-is
>>sue
>>-th
>> en-review. :-)
>
>I think putting it in JIRA (in general) is useful as not every one is 
>on the dev mailing list and gives wider visibility of changes to users 
>of the SDK. It's also easier to search JIRA than the mailing list.
>
>I probably wouldn't of closed the JIRA issue until the change had been 
>submitted into to the SDK but each to their own.  I don't think there 
>have to be one "true" process that everyone must follow.

In my mind a bug should be resolved when fixed, and closed when it makes it
into the relevant branch and has been tested/verified that it works as
expected.

Carol

>
>Thanks,
>Justin



Re: minor Validator improvement

Posted by Carol Frampton <cf...@adobe.com>.

On 3/26/12 7 :07PM, "Justin Mclean" <ju...@classsoftware.com> wrote:

>Hi,
>
>> It is for me.  It is supposed to be commit-then-review, not
>> 
>>commit-then-open-jira-issue-then-resolve-jira-issue-then-close-jira-issue
>>-th
>> en-review. :-) 
>
>I think putting it in JIRA (in general) is useful as not every one is on
>the dev mailing list and gives wider visibility of changes to users of
>the SDK. It's also easier to search JIRA than the mailing list.
>
>I probably wouldn't of closed the JIRA issue until the change had been
>submitted into to the SDK but each to their own.  I don't think there
>have to be one "true" process that everyone must follow.

In my mind a bug should be resolved when fixed, and closed when it makes
it into the relevant branch and has been tested/verified that it works as
expected.

Carol

>
>Thanks,
>Justin


Re: minor Validator improvement

Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,

> It is for me.  It is supposed to be commit-then-review, not
> commit-then-open-jira-issue-then-resolve-jira-issue-then-close-jira-issue-th
> en-review. :-) 

I think putting it in JIRA (in general) is useful as not every one is on the dev mailing list and gives wider visibility of changes to users of the SDK. It's also easier to search JIRA than the mailing list.

I probably wouldn't of closed the JIRA issue until the change had been submitted into to the SDK but each to their own.  I don't think there have to be one "true" process that everyone must follow.

Thanks,
Justin

Re: minor Validator improvement

Posted by Alex Harui <ah...@adobe.com>.


On 3/26/12 12:21 PM, "Jeff Tapper" <je...@spoon.as> wrote:

> Good enough, I created the jira issue as I wasn't sure if the discussion was
> enough, glad to hear it is.
It is for me.  It is supposed to be commit-then-review, not
commit-then-open-jira-issue-then-resolve-jira-issue-then-close-jira-issue-th
en-review. :-) 

I'm sure I'll miss something important someday, but I'd like to keep the
process simple. 

-- 
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui


RE: minor Validator improvement

Posted by Jeff Tapper <je...@spoon.as>.
Good enough, I created the jira issue as I wasn't sure if the discussion was
enough, glad to hear it is.

-----Original Message-----
From: Alex Harui [mailto:aharui@adobe.com] 
Sent: Monday, March 26, 2012 2:51 PM
To: flex-dev@incubator.apache.org
Subject: Re: minor Validator improvement




On 3/26/12 8:46 AM, "Omar Gonzalez" <om...@gmail.com> wrote:


> It sounds like a good idea, my only concern is breaking a bunch of 
> apps that are expecting Validator.validateAll() to return a plain 
> Array. I know there are some methods that were added, instead of 
> changed, to return Vectors as opposed to Arrays and not break existing 
> apps. I can't think of the method name I'm remembering, but I think 
> its one of the display objects that changed. Anyhow, not sure what our 
> approach should be for these kind of changes.
Adobe didn't care if we broke an app if it was for the "greater good".  And
we didn't convert Arrays to Vectors everywhere.  So I don't think there can
be a universal policy.  It has to be considered for each case because there
are trade-offs.  If the array is passed across a MarshallPlan boundary, then
I wouldn't change it.  If the array is likely to be passed back to the
server, then I wouldn't change it as it might screw up AMF serialization.
If the array is a vector of "anything" then I would be cautious.

BTW, I don't think it was necessary to fill out a JIRA issue.  We're
supposed to be watching commit emails and Jeff starting this discussion
should be good enough.

--
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui



Re: minor Validator improvement

Posted by Alex Harui <ah...@adobe.com>.


On 3/26/12 8:46 AM, "Omar Gonzalez" <om...@gmail.com> wrote:


> It sounds like a good idea, my only concern is breaking a bunch of apps
> that are expecting Validator.validateAll() to return a plain Array. I know
> there are some methods that were added, instead of changed, to return
> Vectors as opposed to Arrays and not break existing apps. I can't think of
> the method name I'm remembering, but I think its one of the display objects
> that changed. Anyhow, not sure what our approach should be for these kind
> of changes.
Adobe didn't care if we broke an app if it was for the "greater good".  And
we didn't convert Arrays to Vectors everywhere.  So I don't think there can
be a universal policy.  It has to be considered for each case because there
are trade-offs.  If the array is passed across a MarshallPlan boundary, then
I wouldn't change it.  If the array is likely to be passed back to the
server, then I wouldn't change it as it might screw up AMF serialization.
If the array is a vector of "anything" then I would be cautious.

BTW, I don't think it was necessary to fill out a JIRA issue.  We're
supposed to be watching commit emails and Jeff starting this discussion
should be good enough.

-- 
Alex Harui
Flex SDK Team
Adobe Systems, Inc.
http://blogs.adobe.com/aharui


Re: minor Validator improvement

Posted by Omar Gonzalez <om...@gmail.com>.
On Mon, Mar 26, 2012 at 8:41 AM, Jeff Tapper <je...@spoon.as> wrote:

> In my whiteboard, I just checked in a minor change to the validator class,
> so the ValidateAll method will return a Vector of ValidationResultEvent
> instances, rather than a generic array.    Since the array cant contain
> anything other than ValidationResultEvent instances, a vector makes more
> sense.  Additionally, the strong typing allows for code hinting and compile
> time checking of the results.
>
>
>
> If anyone has a few minutes, I'd appreciate a quick code review and any
> feedback.
>
>
>
> Another change im contemplating is for the argument to be a Vector of
> IValidator instances, rather than a generic array.
>
>
>
> Thoughts?
>
>
It sounds like a good idea, my only concern is breaking a bunch of apps
that are expecting Validator.validateAll() to return a plain Array. I know
there are some methods that were added, instead of changed, to return
Vectors as opposed to Arrays and not break existing apps. I can't think of
the method name I'm remembering, but I think its one of the display objects
that changed. Anyhow, not sure what our approach should be for these kind
of changes.

-omar