Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> Harbs:
 Feel free to do what you want to SonarQube now, but don’t make any changes 
 based on the reports.

I think you would agree that it doesn’t matter what tool you use to find issues 
and a blanket statement like that is like saying you must use a particular IDE 
to edit your code. Sonar cube is a tool that can find bugs and issue in code - 
why not use it to do that?

>>> You are free to review any commits and I make and veto any them on their 
>>> technical merits. You are not free to revert any changes without discussion 
>>> or a veto. Everyone is free to scratch their own itch here and the tools 
>>> you use to find any issues should be irrelevant.

Harbs has recently on several occasions has reverted my commits (usually under 
another check in message) without discussion or a valid veto. I was just 
reminding him that's not the way to do things.

>> Here we have the catch. Apache projects also operate by consensus and as 
>> volunteers. If one individual works against consensus and also takes peoples 
>> time away then problems arise with the community dynamic. RTC has been 
>> suggested precisely because it is one way to solve this issue. There are 
>> other ways.
>> 
>> Consensus is very important please work on that aspect of this discussion.

In this case consensus may not be required on exactly how Sonar Cube is 
configured given most of the rules are minor violations then you can choose to 
ignore them if you please. What would be useful to to have consensus around the 
critical and blocking rules and as you say this may take a little time.

> Think about “other ways”. The project does not appear interested in following 
> RTC.

Which I am. For instance it looks like there wasn’t full consensus on using 
hard coded string vs constants for event names so I suggested having a vote on 
it.

Thanks,
Justin

Re: git commit: [flex-asjs] [refs/heads/develop] - added simple positioning to tool tip / make tool tip not react to mouse events

[Justin Mclean]

Hi,

Here you go:

diff --git 
a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/accessories/ToolTipBead.as
 
b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/accessories/ToolTipBead.as
index 3ccf708..89e89c4 100644
--- 
a/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/accessories/ToolTipBead.as
+++ 
b/frameworks/projects/Basic/src/main/flex/org/apache/flex/html/accessories/ToolTipBead.as
@@ -53,17 +53,11 @@ package org.apache.flex.html.accessories
{
}

-   public static const TOP:int = 1;
-   public static const BOTTOM:int = 10001;
-   public static const LEFT:int = 10002;
-   public static const RIGHT:int = 10003;
-   public static const MIDDLE:int = 10004;
-
private var _toolTip:String;
+   private var _strand:IStrand;
private var tt:ToolTip;
private var host:IPopUpHost;
-   private var _xPos:int = RIGHT;
-   private var _yPos:int = BOTTOM;
+   private var _position:String = "bottom right";

/**
 *  The string to use as the toolTip.
@@ -83,35 +77,22 @@ package org.apache.flex.html.accessories
}

/**
-*  Sets the tooltip y relative position to one of
-*  LEFT, MIDDLE or RIGHT.
+*  The class styles used to position the toolTip.
 *
 *  @langversion 3.0
 *  @playerversion Flash 10.2
 *  @playerversion AIR 2.6
 *  @productversion FlexJS 0.9
 */
-   public function set xPos(pos:int):void
+   public function get position():String
{
-   _xPos = pos;
+   return _position;
}
-
-   /**
-*  Sets the tooltip y relative position to one of
-*  TOP, MIDDLE or BOTTOM.
-*
-*  @langversion 3.0
-*  @playerversion Flash 10.2
-*  @playerversion AIR 2.6
-*  @productversion FlexJS 0.9
-*/
-   public function set yPos(pos:int):void
+   public function set position(value:String):void
{
-   _yPos = pos;
+   _position = value;
}

-   private var _strand:IStrand;
-
/** 
 *  @copy org.apache.flex.core.IBead#strand
 *
@@ -144,8 +125,24 @@ package org.apache.flex.html.accessories
tt = new ToolTip();
tt.text = toolTip;
var pt:Point = determinePosition(event, event.target);
-tt.x = pt.x;
-tt.y = pt.y;
+   if (pt) {
+   tt.x = pt.x;
+   tt.y = pt.y;
+   }
+   else {
+   COMPILE::SWF
+   {
+   if (!tt["styleName"])  {
+   tt["styleName"] = position;
+   }
+   }
+   COMPILE::JS
+   {
+   if (!tt.className) {
+   tt.className = position;
+   }
+   }
+   }
host.addElement(tt, false); // don't trigger a layout
}

@@ -155,35 +152,9 @@ package org.apache.flex.html.accessories
 */
protected function determinePosition(event:MouseEvent, 
base:Object):Point
{
-   var comp:IUIBase = _strand as IUIBase;
-   var xFactor:Number = 1;
-   var yFactor:Number = 1;
-   var pt:Point;
-   var relative:Boolean = _xPos > TOP &&  _yPos > TOP;
-
-   if (_xPos == LEFT) {
-   xFactor = Number.POSITIVE_INFINITY;
-   }
-   else if (_xPos == MIDDLE) {
-   xFactor = 2;
-   }
-   else if (_xPos == RIGHT) {
-   xFactor = 1;
-   }
-   if (_yPos == TOP) {
-   yFactor = Number.POSITIVE_INFINITY;
-   }
-   else if (_yPos == MIDDLE) {
-   yFactor = 2;
-   }
-   else if (_yPos == BOTTOM) {
-   

Re: [FlexJS] Debugging package

[Josh Tynjala]

I'm working on adding support for the debugger statement to the compiler
(FLEX-35343). I can successfully emit the debugger statement in the
generated JS so far.

I'm not yet sure if I can make it work on the SWF side. I figured out where
I can generate bytecode instructions in ABCGeneratingReducer. If I can
figure out how to generate bytecode to call enterDebugger() (or whatever
the function is called), the debugger statement will work in both SWF and
JS.

- Josh

On Sun, Jul 16, 2017 at 9:22 AM, Josh Tynjala  wrote:

> If it were a variable or function, it could be defined somewhere like
> that. It's a statement, though, so it needs to be added to where Falcon
> creates the AST from the ActionScript code. Then, it also needs to emit the
> statement as JS in FalconJX. On the SWF side, it should be translated to
> appropriate bytecode to call enterDebugger().
>
> - Josh
>
> On Jul 16, 2017 8:56 AM, "Harbs"  wrote:
>
>> What needs to be modified? Does it need to be added to NativeJSType
>> enums? Somewhere else?
>>
>> I’m really not clear on when things to be added to that and when they
>> need to be in typedefs.
>>
>> > On Jul 16, 2017, at 6:51 PM, Harbs  wrote:
>> >
>> >> The compiler needs to be modified to support the debugger statement.
>>
>>

Re: [FlexJS] technical debt

[Dave Fisher]

Justin -

You seem to pick the part of the conversation that you are interested in and 
edit out the rest of the context. It makes it difficult to have a conversation 
about the best way to move forward in this project.

Harbs:
>>> Feel free to do what you want to SonarQube now, but don’t make any changes 
>>> based on the reports.
>> 

Justin:
>> You are free to review any commits and I make and veto any them on their 
>> technical merits. You are not free to revert any changes without discussion 
>> or a veto. Everyone is free to scratch their own itch here and the tools you 
>> use to find any issues should be irrelevant.

Dave:
> Here we have the catch. Apache projects also operate by consensus and as 
> volunteers. If one individual works against consensus and also takes peoples 
> time away then problems arise with the community dynamic. RTC has been 
> suggested precisely because it is one way to solve this issue. There are 
> other ways.
> 
> Consensus is very important please work on that aspect of this discussion.

My whole statement worked together and I would appreciate it if you would think 
about this in its whole. Think about other times when you and Harbs have had 
this dynamic.

Think about “other ways”. The project does not appear interested in following 
RTC.

Ahh there went 10 minutes to try to explain it again. (Sigh)

Regards,
Dave


> 
> Thank you,
> Dave
> 



signature.asc
Description: Message signed with OpenPGP

Re: [FlexJS XML] for each

[Justin Mclean]

Hi,

> I assume you tested this with regular Flex and not the Falcon compiler?  I
> just want to verify a couple of your findings.

Be aware there are existing unresolved bugs in the Flex SDK in this area i.e. 
[1]

Thanks,
Justin

1. https://issues.apache.org/jira/browse/FLEX-33644

Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> The exercise with the Wiki was essentially for you to present a proposed set 
> of Sonar rules. You did this, thank you.
> 
> It was not intended by me as [LAZY CONSENSUS]. Clearly there is healthy 
> disagreement and consensus is not yet achieved.

Nor is it intended to be. I created a new rule set so people can see the 
results of what is documented on that wiki page. People are welcome to provide 
further feedback.

> You can see that a lot ion Flex Developers do not agree. Which ones is a good 
> question, but keep in mind this will take time, and should not be forced. I 
> think that the burden should be a discussion of a specific case.

Which is what we're doing the the cloneEvent method.

> RTC has been suggested precisely because it is one way to solve this issue.

Which I would be in favour of.  

Thanks,
Justin

Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> Also, in FlexJS, we want very few if any custom events to bubble.
> Bubbling was overused in regular Flex and was, IMO, a bad practice because
> it breaks encapsulation.

Events provide encapsulation by allowing application to respond to changes 
outside of the object. IMO it’s also a good practice because it encourages 
loose coupling.

If we shouldn't be using bubbling in FlexJS why does just about event Event (30 
odd) have a bubbling attribute and more than 3/4 have a cloneEvent method? 
Either we shod be fixing the broken one or removing the methods on the existing 
event classes - which is it?

Thanks,
Justin

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[Justin Mclean]

Hi,

> Can we focus more on the priorities from the summit [1]?  

Sonar Cube analysis was discussed at the summit i.e. items 5 and 6 which it can 
help with.

> Or maybe a feature like AMF that some folks really need?

I’m not interested in AMF support so it’s unlikely I’ll work on it.

Thanks,
Justin

Re: [FlexJS] String.match()

[Harbs]

Here seems to be the explanation for that:

new RegExp("?”)

In Flash, this creates a RegExp object.

In JS, it throws an “Invalid regular expression” error.

> On Jul 18, 2017, at 9:38 PM, Harbs  wrote:
> 
> The “?” case seems to be an outlier.

Re: [FlexJS] String.match()

[yishayw]

Alex Harui-2 wrote
>  By
> calling new utility functions, the developer has control over the
> conversion.

I don't understand that point. Do you mean an app developer?




--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/FlexJS-String-match-tp63392p63405.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: [FlexJS] String.match()

[Harbs]

Calling language functions at first seemed unnecessarily expensive to me.

I just did a bit of research, and it looks like I was wrong about replacing 
search with indexOf. That will only work for simple string searches. 
str.search(“.”) will always return 0 unless the string is empty, because the 
“.” is evaluated to new RegExp(“.”). This is true both in Flash and JS 
environments. The “?” case seems to be an outlier. Either way, I’m not sure how 
my code ever worked. str.match(“?”) seems to always return null.

Language functions seem like they are more expensive, but you might be right 
that it might be the best way to go.

The implementation should probably use a try/catch to construct a RegExp object 
from the string and return null/-1 if the constructor causes an error. That 
would be the same as the Flash implementation. We could also use debug.warn() 
to let the developer know that using strings is not recommended.

Harbs

> On Jul 18, 2017, at 6:37 PM, Alex Harui  wrote:
> 
> This feels like something that the compiler should just call
> Language.stringMatch and Language.stringSearch (or standalone
> equivalents).  I'm not clear that we should always modify the string.  By
> calling new utility functions, the developer has control over the
> conversion.
> 
> -Alex
> 
> On 7/18/17, 4:11 AM, "Harbs"  > wrote:
> 
>> The same issue applies to String.search(), although it might make sense
>> to replace String.search() with String.indexOf() if the parameter is a
>> string.
>> 
>>> On Jul 18, 2017, at 12:36 PM, yishayw  wrote:
>>> 
>>> In flash String.match() can take either a string or a regex. In JS it's
>>> always considered to be a regex. So str.match("?") is valid in flash but
>>> will fail in JS. We ran into that porting our app which includes some
>>> code
>>> to test for url params.
>>> 
>>> So maybe the compiler should identify that the input is a string and
>>> replace
>>> all special chars as suggested here [1]?
>>> 
>>> [1]
>>> 
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstackove 
>>> 
>>> rflow.com 
>>> %2Fquestions%2F3561493%2Fis-there-a-regexp-escape-function-in-ja
>>> vascript=02%7C01%7C%7C2260cdfd26d74274ba4a08d4cdcde8ea%7Cfa7b1b5a7b3
>>> 4438794aed2c178decee1%7C0%7C0%7C636359731646059595=LKSeBWNfrL6CCQRg
>>> eMI48HJSvACdKoNp5X3AZuMoZeU%3D=0
>>> 
>>> 
>>> 
>>> 
>>> --
>>> View this message in context:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fapache-fl 
>>> 
>>> ex-development.247.n4.nabble.com 
>>> %2FFlexJS-String-match-tp63392.html
>>> ata=02%7C01%7C%7C2260cdfd26d74274ba4a08d4cdcde8ea%7Cfa7b1b5a7b34438794aed
>>> 2c178decee1%7C0%7C0%7C636359731646069605=X0ZlS9B3AUiglpM5ywHSi4aqui
>>> jF55mWfBbbEfnJE6E%3D=0
>>> Sent from the Apache Flex Development mailing list archive at
>>> Nabble.com .

Re: [FlexJS XML] for each

[Harbs]

> On Jul 18, 2017, at 6:13 PM, Alex Harui  wrote:
> 
> I assume you tested this with regular Flex and not the Falcon compiler?

Yes.

> I just want to verify a couple of your findings.
> 
> 1) Does passing empty string really produce an XML object with no children
> or is there a child textNode with “”?

new XML() and new XML(null) and new XML(“”) all produce identical XML objects. 
By default, XML seems to have a nodeKind of text. If you set properties on the 
XML, that changes. If you pass a string, it creates a text node with no 
children:

The following:

var xml7:XML = new XML();
var xml8:XML = new XML("");
var xml9:XML = new XML(null);
var xml10:XML = new XML("foo");
var xml11:XML = new XML({foo:"baz"});
trace(xml7.children().length());
trace(xml8.children().length());
trace(xml9.children().length());
trace(xml10.children().length());
trace(xml11.children().length());
trace(xml7);
trace(xml8);
trace(xml9);
trace(xml10);
trace(xml11);

Outputs:

0
0
0
0
0



foo
[object Object]

> 2) If you pass in an XMLList with one element, doesn't
> 
>  var harbs:XML = XML(someXMLListWithOneElement)
> 
> return the one element but
> 
>  var harbs:XML = new XML(someXMLListWithOneElement)
> 
> return a clone?

Good call.

The following code:

var xml1:XML = new XML("foo");
var xml2:XML = XML(xml1);
var xml3:XML = new XML(xml1);
trace(xml1 === xml2);
trace(xml1 === xml3);
var xml4:XMLList = new XMLList(xml1);
var xml5:XML = new XML(xml4);
var xml6:XML = XML(xml4);
trace(xml1 === xml5);
trace(xml1 === xml6);
trace(xml5 === xml6);

Outputs:

true
false
false
true
false

(“==“ outputs true for all of the above.)

>   
> IMO, toXML should do what Flash does unless it is really expensive.  If
> you had code that relied on Flash behavior, what would you have to do to
> rework the code to the spec?
> 

I think the observed behavior is bug-prone. The documented behavior makes much 
more sense. I can’t think of a good reason to cast null or undefined to XML. If 
you really want to cast on object to a string, you can always call toString(). 
Fixing such code should be really easy.

In terms of implementation, it’s not really harder one way or the other.

> My 2 cents,
> -Alex
> 
> On 7/18/17, 1:29 AM, "Harbs"  > wrote:
> 
>> Actually, it’s not really necessary to allow null and undefined to get an
>> empty XML object. The constructor can default to en empty string. So an
>> empty string could get an XML object while null and undefined could throw
>> an error.
>> 
>> That might make more sense because it would likely catch more errors.
>> 
>>> On Jul 18, 2017, at 11:07 AM, Harbs  wrote:
>>> 
>>> I discovered that the documentation on the top level functions is wrong.
>>> 
>>> According to the docs[1] the only valid expressions for XML and XMLList
>>> are: XML, XMLList, Boolean, Number and String. Anything else is supposed
>>> to throw an error.
>>> 
>>> What actually happens is that null and undefined are simply swallowed
>>> and you get an empty XML text node object. Everything else simply has
>>> toString() called on it, so if you pass in an Object which does not have
>>> an implementation of toString(), you’ll get a text node with a value of
>>> [object Object].
>>> 
>>> In my tests, new XML() and XML() behave identically.
>>> 
>>> That’s not what I’d call great behavior…
>>> 
>>> So the question is what to do. What I think makes sense is to make
>>> toXML() behave like the docs and new XML() behave like the observed
>>> behavior. At the least for null and undefined. There needs to be a way
>>> to instantiate an empty XML object. I’m on the fence about objects. I’m
>>> leaning toward always throwing an error if the argument is an object.
>>> 
>>> Thoughts?
>>> 
>>> Harbs
>>> 
>>> 
>>> [1]https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fhelp.a 
>>> 
>>> dobe.com 
>>> %2Fen_US%2FFlashPlatform%2Freference%2Factionscript%2F3%2Fpackage
>>> .html%23XML=02%7C01%7C%7C9ab55b27d96c403de2e208d4cdb7a394%7Cfa7b1b5a
>>> 7b34438794aed2c178decee1%7C0%7C0%7C636359635983774211=rxcx9eUoc%2F%
>>> 2FufMzesTKqS8fp7bdV1yQUIoPyckupXGs%3D=0()
>>> >> 
>>> be.com 
>>> %2Fen_US%2FFlashPlatform%2Freference%2Factionscript%2F3%2Fpackage.h
>>> tml%23XML=02%7C01%7C%7C9ab55b27d96c403de2e208d4cdb7a394%7Cfa7b1b5a7b
>>> 34438794aed2c178decee1%7C0%7C0%7C636359635983774211=rxcx9eUoc%2F%2F
>>> ufMzesTKqS8fp7bdV1yQUIoPyckupXGs%3D=0()>
>>> 
 On Jul 18, 2017, at 8:20 AM, Harbs 
 >> wrote:
 
 I’ll try to write these functions today.

Re: [FlexJS] technical debt

[Dave Fisher]

Hi Justin,

The exercise with the Wiki was essentially for you to present a proposed set of 
Sonar rules. You did this, thank you.

It was not intended by me as [LAZY CONSENSUS]. Clearly there is healthy 
disagreement and consensus is not yet achieved.

> On Jul 17, 2017, at 11:22 PM, Justin Mclean  wrote:



> 
> Would you not agree that some (if not all) of these need fixing?
> 
>> A lot of the rules fly in the face to current convention in the SDK.
> 
> Which ones in particular?

You can see that a lot ion Flex Developers do not agree. Which ones is a good 
question, but keep in mind this will take time, and should not be forced. I 
think that the burden should be a discussion of a specific case.

> 
>> Some (such as returning from a constructor) are actually enforced by the 
>> compiler so the rule is not needed.
> 
> If that the case there no harm in having them on as there should be no 
> violations right?

The harm can come as seen bellow.

> 
>> Feel free to do what you want to SonarQube now, but don’t make any changes 
>> based on the reports.
> 
> You are free to review any commits and I make and veto any them on their 
> technical merits. You are not free to revert any changes without discussion 
> or a veto. Everyone is free to scratch their own itch here and the tools you 
> use to find any issues should be irrelevant.


Here we have the catch. Apache projects also operate by consensus and as 
volunteers. If one individual works against consensus and also takes peoples 
time away then problems arise with the community dynamic. RTC has been 
suggested precisely because it is one way to solve this issue. There are other 
ways.

Consensus is very important please work on that aspect of this discussion.

Thank you,
Dave



signature.asc
Description: Message signed with OpenPGP

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[piotrz]

Alex,
 +1
Thanks for such reminder! Really wish someone could start look into that!

Piotr



-
Apache Flex PMC
piotrzarzyck...@gmail.com
--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-Added-support-for-JS-upload-progress-events-tp63275p63400.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: [FlexJS] String.match()

[Alex Harui]

This feels like something that the compiler should just call
Language.stringMatch and Language.stringSearch (or standalone
equivalents).  I'm not clear that we should always modify the string.  By
calling new utility functions, the developer has control over the
conversion.

-Alex

On 7/18/17, 4:11 AM, "Harbs"  wrote:

>The same issue applies to String.search(), although it might make sense
>to replace String.search() with String.indexOf() if the parameter is a
>string.
>
>> On Jul 18, 2017, at 12:36 PM, yishayw  wrote:
>> 
>> In flash String.match() can take either a string or a regex. In JS it's
>> always considered to be a regex. So str.match("?") is valid in flash but
>> will fail in JS. We ran into that porting our app which includes some
>>code
>> to test for url params.
>> 
>> So maybe the compiler should identify that the input is a string and
>>replace
>> all special chars as suggested here [1]?
>> 
>> [1]
>> 
>>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstackove
>>rflow.com%2Fquestions%2F3561493%2Fis-there-a-regexp-escape-function-in-ja
>>vascript=02%7C01%7C%7C2260cdfd26d74274ba4a08d4cdcde8ea%7Cfa7b1b5a7b3
>>4438794aed2c178decee1%7C0%7C0%7C636359731646059595=LKSeBWNfrL6CCQRg
>>eMI48HJSvACdKoNp5X3AZuMoZeU%3D=0
>> 
>> 
>> 
>> 
>> --
>> View this message in context:
>>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fapache-fl
>>ex-development.247.n4.nabble.com%2FFlexJS-String-match-tp63392.html
>>ata=02%7C01%7C%7C2260cdfd26d74274ba4a08d4cdcde8ea%7Cfa7b1b5a7b34438794aed
>>2c178decee1%7C0%7C0%7C636359731646069605=X0ZlS9B3AUiglpM5ywHSi4aqui
>>jF55mWfBbbEfnJE6E%3D=0
>> Sent from the Apache Flex Development mailing list archive at
>>Nabble.com.
>

Re: [FlexJS] technical debt

[Josh Tynjala]

Additionally, the cloning results in extra objects that need to be garbage
collected, which can hurt performance and make effects/animations look bad.
Good call on avoiding it!

- Josh

On Tue, Jul 18, 2017 at 8:23 AM, Alex Harui 
wrote:

> +1 to what Harbs said.
>
> Also, in FlexJS, we want very few if any custom events to bubble.
> Bubbling was overused in regular Flex and was, IMO, a bad practice because
> it breaks encapsulation.
>
> Bubbling was intended to allow an object in the DOM to monitor inputs to
> its children without having to attach listeners to the entire tree.  It
> was not meant as a way for a component to shout to the world "Hey, I just
> changed a property, does anybody care?"
>
> -Alex
>
> On 7/17/17, 11:05 PM, "Harbs"  wrote:
>
> >event.clone() is wrong for FlexJS. It’s event.cloneEvent()
> >
> >There was other feedback on the dev list. That seemed to have been
> >ignored.
> >
> >A lot of the rules fly in the face to current convention in the SDK. Some
> >(such as returning from a constructor) are actually enforced by the
> >compiler so the rule is not needed. I don’t have time right now to get
> >involved in a long protracted discussion, but please don’t take the lack
> >of input as consensus.
> >
> >Feel free to do what you want to SonarQube now, but don’t make any
> >changes based on the reports. If anyone else takes an interest in it,
> >please expect some rules to be changed.
> >
> >Thanks,
> >Harbs
> >
> >> On Jul 18, 2017, at 8:50 AM, Justin Mclean 
> >>wrote:
> >>
> >> Hi,
> >>
> >> So more than a week has gone by and I’ve received little feedback on
> >>[1]. Given that I’ll gone ahead and implement the rules as discussed in
> >>the document so people can see the changes.
> >>
> >> The new results are up:
> >> - First off the header rules is overly strict and expects a copyright
> >>line not just a header. ASF policy to not to have copyright lines so
> >>this is a false positive. I’ll disable the rule. We have other means to
> >>check headers. This will be fixed in the next run.
> >> - The ASDocs rule is picking a lot of public methods and properties
> >>that don’t have ASDocs. I think we should keep this to remind us at some
> >>point it needs to be fixed / people can fix as they make edits.
> >> - Is easier to see if issues that do need to be fixed. For instance I
> >>can see that there are a couple of event.clone issues that are likely
> >>bugs.
> >>
> >> Thanks,
> >> Justin
> >>
> >> 1.
> >>https://na01.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fcwiki.ap
> >>ache.org%2Fconfluence%2Fdisplay%2FFLEX%2FSonar%
> 2BCube%2BFlex%2BRules
> >>=02%7C01%7C%7C9be74a7882374975005208d4cda30749%
> 7Cfa7b1b5a7b34438794aed2c1
> >>78decee1%7C0%7C0%7C636359547464506542=
> cPs4sqimu5YFG%2B08ymZNtb9HMWC
> >>WFGixhtyGrP0burM%3D=0
> >> https%3A%2F%2Fcwiki.a
> >>pache.org%2Fconfluence%2Fdisplay%2FFLEX%2FSonar%
> 2BCube%2BFlex%2BRules
> >>a=02%7C01%7C%7C9be74a7882374975005208d4cda3
> 0749%7Cfa7b1b5a7b34438794aed2c
> >>178decee1%7C0%7C0%7C636359547464506542=
> cPs4sqimu5YFG%2B08ymZNtb9HMW
> >>CWFGixhtyGrP0burM%3D=0>
> >
>
>

Re: [FlexJS] technical debt

[Alex Harui]

+1 to what Harbs said.

Also, in FlexJS, we want very few if any custom events to bubble.
Bubbling was overused in regular Flex and was, IMO, a bad practice because
it breaks encapsulation.

Bubbling was intended to allow an object in the DOM to monitor inputs to
its children without having to attach listeners to the entire tree.  It
was not meant as a way for a component to shout to the world "Hey, I just
changed a property, does anybody care?"

-Alex

On 7/17/17, 11:05 PM, "Harbs"  wrote:

>event.clone() is wrong for FlexJS. It’s event.cloneEvent()
>
>There was other feedback on the dev list. That seemed to have been
>ignored.
>
>A lot of the rules fly in the face to current convention in the SDK. Some
>(such as returning from a constructor) are actually enforced by the
>compiler so the rule is not needed. I don’t have time right now to get
>involved in a long protracted discussion, but please don’t take the lack
>of input as consensus.
>
>Feel free to do what you want to SonarQube now, but don’t make any
>changes based on the reports. If anyone else takes an interest in it,
>please expect some rules to be changed.
>
>Thanks,
>Harbs
>
>> On Jul 18, 2017, at 8:50 AM, Justin Mclean 
>>wrote:
>> 
>> Hi,
>> 
>> So more than a week has gone by and I’ve received little feedback on
>>[1]. Given that I’ll gone ahead and implement the rules as discussed in
>>the document so people can see the changes.
>> 
>> The new results are up:
>> - First off the header rules is overly strict and expects a copyright
>>line not just a header. ASF policy to not to have copyright lines so
>>this is a false positive. I’ll disable the rule. We have other means to
>>check headers. This will be fixed in the next run.
>> - The ASDocs rule is picking a lot of public methods and properties
>>that don’t have ASDocs. I think we should keep this to remind us at some
>>point it needs to be fixed / people can fix as they make edits.
>> - Is easier to see if issues that do need to be fixed. For instance I
>>can see that there are a couple of event.clone issues that are likely
>>bugs.
>> 
>> Thanks,
>> Justin
>> 
>> 1.  
>>https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.ap
>>ache.org%2Fconfluence%2Fdisplay%2FFLEX%2FSonar%2BCube%2BFlex%2BRules
>>=02%7C01%7C%7C9be74a7882374975005208d4cda30749%7Cfa7b1b5a7b34438794aed2c1
>>78decee1%7C0%7C0%7C636359547464506542=cPs4sqimu5YFG%2B08ymZNtb9HMWC
>>WFGixhtyGrP0burM%3D=0
>>>pache.org%2Fconfluence%2Fdisplay%2FFLEX%2FSonar%2BCube%2BFlex%2BRules
>>a=02%7C01%7C%7C9be74a7882374975005208d4cda30749%7Cfa7b1b5a7b34438794aed2c
>>178decee1%7C0%7C0%7C636359547464506542=cPs4sqimu5YFG%2B08ymZNtb9HMW
>>CWFGixhtyGrP0burM%3D=0>
>

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[Alex Harui]

Justin, this is not worth the time being spent on it.

Can we focus more on the priorities from the summit [1]?  Or maybe a
feature like AMF that some folks really need?

[1] 
https://lists.apache.org/thread.html/3ef2cc9fcd17bbf0c81b38c35586790255277f
bb4f727db8882920ec@%3Cdev.flex.apache.org%3E

-Alex

On 7/17/17, 11:28 PM, "Justin Mclean"  wrote:

>Hi,
>
>> No. The consensus was that the compiler needs improvement to replace
>>static constants with string literals and for now to not enforce one way
>>or the other.
>
>I’m not sure how you get that from the two threads he had on this. I’m
>happy to go through and summaries the thread for you if you want or if
>you prefer let call a VOTE and abide by the results of that?
>
>> Until the compiler is fixed, my personal preference remains to use
>>string literals.
>
>You personal preference that may be that but you changed code so that it
>not longer uses constants.
>
>>> I also note the code is using "”+requestStatus to convert a number to
>>>a string. Any reason for not using the toString or
>>>String(requestStatus) instead?
>> 
>> It’s more concise.
>
>It’s also buggy (for large numbers for instance) so I would take care in
>using it, if you were worried about null or undefined then
>String(requestStatus) will do what you need and is much clearer to
>understand.
>
>Thanks,
>Justin

Re: [FlexJS XML] for each

[Alex Harui]

I assume you tested this with regular Flex and not the Falcon compiler?  I
just want to verify a couple of your findings.

1) Does passing empty string really produce an XML object with no children
or is there a child textNode with ""?

2) If you pass in an XMLList with one element, doesn't

  var harbs:XML = XML(someXMLListWithOneElement)

return the one element but

  var harbs:XML = new XML(someXMLListWithOneElement)

return a clone?

IMO, toXML should do what Flash does unless it is really expensive.  If
you had code that relied on Flash behavior, what would you have to do to
rework the code to the spec?

My 2 cents,
-Alex

On 7/18/17, 1:29 AM, "Harbs"  wrote:

>Actually, it’s not really necessary to allow null and undefined to get an
>empty XML object. The constructor can default to en empty string. So an
>empty string could get an XML object while null and undefined could throw
>an error.
>
>That might make more sense because it would likely catch more errors.
>
>> On Jul 18, 2017, at 11:07 AM, Harbs  wrote:
>> 
>> I discovered that the documentation on the top level functions is wrong.
>> 
>> According to the docs[1] the only valid expressions for XML and XMLList
>>are: XML, XMLList, Boolean, Number and String. Anything else is supposed
>>to throw an error.
>> 
>> What actually happens is that null and undefined are simply swallowed
>>and you get an empty XML text node object. Everything else simply has
>>toString() called on it, so if you pass in an Object which does not have
>>an implementation of toString(), you’ll get a text node with a value of
>>[object Object].
>> 
>> In my tests, new XML() and XML() behave identically.
>> 
>> That’s not what I’d call great behavior…
>> 
>> So the question is what to do. What I think makes sense is to make
>>toXML() behave like the docs and new XML() behave like the observed
>>behavior. At the least for null and undefined. There needs to be a way
>>to instantiate an empty XML object. I’m on the fence about objects. I’m
>>leaning toward always throwing an error if the argument is an object.
>> 
>> Thoughts?
>> 
>> Harbs
>> 
>> 
>>[1]https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fhelp.a
>>dobe.com%2Fen_US%2FFlashPlatform%2Freference%2Factionscript%2F3%2Fpackage
>>.html%23XML=02%7C01%7C%7C9ab55b27d96c403de2e208d4cdb7a394%7Cfa7b1b5a
>>7b34438794aed2c178decee1%7C0%7C0%7C636359635983774211=rxcx9eUoc%2F%
>>2FufMzesTKqS8fp7bdV1yQUIoPyckupXGs%3D=0()
>>>be.com%2Fen_US%2FFlashPlatform%2Freference%2Factionscript%2F3%2Fpackage.h
>>tml%23XML=02%7C01%7C%7C9ab55b27d96c403de2e208d4cdb7a394%7Cfa7b1b5a7b
>>34438794aed2c178decee1%7C0%7C0%7C636359635983774211=rxcx9eUoc%2F%2F
>>ufMzesTKqS8fp7bdV1yQUIoPyckupXGs%3D=0()>
>> 
>>> On Jul 18, 2017, at 8:20 AM, Harbs >>> wrote:
>>> 
>>> I’ll try to write these functions today.
>>> 
>> 
>

Re: [FlexJS] findPopupHost issue

[Alex Harui]

I guess I should have been more clear.  FlexJS supports multiple component
sets and runtimes.  An Application class is probably always going to be
the entry point of an application, but there is no guarantee in any
application that the Application class will be a display object or even
the correct display object in the case of a multi-window desktop
application.

IPopUpHost is an API for a good place to hang popups, but it is only a
coincidence that in the currently supported runtimes and most popular
component sets that the Application is mapped to a display object and thus
an IPopUpHost.  We should not presume that will always be the case.

If you are looking for a good place to hang a popup you probably want it
to popup over some UI component.  So, it is best to start with that known
IUIBase and find an IPopUpHost that will display the popup over that
IUIBase.  That way, your code will work in a multi-window app someday, and
other future runtime environments as well.  And that's why findPopUpHost
takes an IUIBase.  And in other future runtimes, it should be implemented
to return a useful thing, but that thing may not be the Application
instance.

So, the recommended practice is to use UIUtils.findPopupHost and pass it
an IUIBase that you want the popup to float over.

HTH,
-Alex

On 7/17/17, 10:48 PM, "Yishay Weiss"  wrote:

>Even if the Application instance isn’t an IPopUpHost the method will just
>return null, so the effect is the same as just doing (appInstance as
>IPopUpHost). I suspect the method is being misused rather than there
>being a problem with the method.
>
>From: Alex Harui
>Sent: Tuesday, July 18, 2017 8:33 AM
>To: dev@flex.apache.org
>Subject: Re: [FlexJS] findPopupHost issue
>
>FWIW, it may not be safe to assume that Application will always be an
>IPopUpHost, hence the utility function.
>
>Thanks,
>-Alex
>
>On 7/17/17, 9:59 PM, "piotrz"  wrote:
>
>>Hi Yishay,
>>
>>That's a good point - Application is itself an IPopupHost. I've asked him
>>what is the use case for his host popup search.
>>
>>Thanks,
>>Piotr
>>
>>
>>
>>-
>>Apache Flex PMC
>>piotrzarzyck...@gmail.com
>>--
>>View this message in context:
>>https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fapache-fl
>>e
>>x-development.247.n4.nabble.com%2FFlexJS-findPopupHost-issue-tp63362p
>>6
>>3366.html=02%7C01%7C%7Cbfd19d2deba646c64df208d4cd9c9798%7Cfa7b1b5a7b
>>3
>>4438794aed2c178decee1%7C0%7C0%7C636359519816386890=CyeQuoJ0gVVl2UHU
>>B
>>W%2B6hbyDNw%2FOSFuswfzeLTqUf3c%3D=0
>>Sent from the Apache Flex Development mailing list archive at Nabble.com.
>

Re: git commit: [flex-asjs] [refs/heads/develop] - added simple positioning to tool tip / make tool tip not react to mouse events

[Alex Harui]

Maybe you are taking the wrong approach.  Feel free to share your proposed
changes in a branch or as a patch.

-Alex

On 7/17/17, 9:49 PM, "Justin Mclean"  wrote:

>Hi,
>
>So I’ve looked into it and have rough version code up and so far:
>- Requires support of styleName in AS which AFAICS is missing and/or not
>working. Without this 9 CSS classes are needed rather than 6 CSS classes.
>- DataToolTipBead overrides determinePosition so the base function is
>still required returning null and rollOverHandler needs checking that
>return (i.e. more just in case code)
>- More just in case code is need on JS and AS to check if there is a
>class sector in use and if not default to the “bottom right” one.
>- CSS is not optimised by the optimiser, the code is.
>
>While it may be more flexible it's is ending up fatter / has more just in
>case code that the current checked in version so I don’t think it a
>solution for this bead. It may be for a more heavy weight one.
>
>Thanks,
>Justin
>
>

Re: [FlexJS] String.match()

[Harbs]

The same issue applies to String.search(), although it might make sense to 
replace String.search() with String.indexOf() if the parameter is a string.

> On Jul 18, 2017, at 12:36 PM, yishayw  wrote:
> 
> In flash String.match() can take either a string or a regex. In JS it's
> always considered to be a regex. So str.match("?") is valid in flash but
> will fail in JS. We ran into that porting our app which includes some code
> to test for url params. 
> 
> So maybe the compiler should identify that the input is a string and replace
> all special chars as suggested here [1]?
> 
> [1]
> https://stackoverflow.com/questions/3561493/is-there-a-regexp-escape-function-in-javascript
> 
> 
> 
> 
> --
> View this message in context: 
> http://apache-flex-development.247.n4.nabble.com/FlexJS-String-match-tp63392.html
> Sent from the Apache Flex Development mailing list archive at Nabble.com.

[FlexJS] String.match()

[yishayw]

In flash String.match() can take either a string or a regex. In JS it's
always considered to be a regex. So str.match("?") is valid in flash but
will fail in JS. We ran into that porting our app which includes some code
to test for url params. 

So maybe the compiler should identify that the input is a string and replace
all special chars as suggested here [1]?

[1]
https://stackoverflow.com/questions/3561493/is-there-a-regexp-escape-function-in-javascript
 



--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/FlexJS-String-match-tp63392.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: [FlexJS XML] for each

[Harbs]

Actually, it’s not really necessary to allow null and undefined to get an empty 
XML object. The constructor can default to en empty string. So an empty string 
could get an XML object while null and undefined could throw an error.

That might make more sense because it would likely catch more errors.

> On Jul 18, 2017, at 11:07 AM, Harbs  wrote:
> 
> I discovered that the documentation on the top level functions is wrong.
> 
> According to the docs[1] the only valid expressions for XML and XMLList are: 
> XML, XMLList, Boolean, Number and String. Anything else is supposed to throw 
> an error.
> 
> What actually happens is that null and undefined are simply swallowed and you 
> get an empty XML text node object. Everything else simply has toString() 
> called on it, so if you pass in an Object which does not have an 
> implementation of toString(), you’ll get a text node with a value of [object 
> Object].
> 
> In my tests, new XML() and XML() behave identically.
> 
> That’s not what I’d call great behavior…
> 
> So the question is what to do. What I think makes sense is to make toXML() 
> behave like the docs and new XML() behave like the observed behavior. At the 
> least for null and undefined. There needs to be a way to instantiate an empty 
> XML object. I’m on the fence about objects. I’m leaning toward always 
> throwing an error if the argument is an object.
> 
> Thoughts?
> 
> Harbs
> 
> [1]http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/package.html#XML()
>  
> 
> 
>> On Jul 18, 2017, at 8:20 AM, Harbs > > wrote:
>> 
>> I’ll try to write these functions today.
>> 
> 

Re: [FlexJS XML] for each

[Harbs]

I discovered that the documentation on the top level functions is wrong.

According to the docs[1] the only valid expressions for XML and XMLList are: 
XML, XMLList, Boolean, Number and String. Anything else is supposed to throw an 
error.

What actually happens is that null and undefined are simply swallowed and you 
get an empty XML text node object. Everything else simply has toString() called 
on it, so if you pass in an Object which does not have an implementation of 
toString(), you’ll get a text node with a value of [object Object].

In my tests, new XML() and XML() behave identically.

That’s not what I’d call great behavior…

So the question is what to do. What I think makes sense is to make toXML() 
behave like the docs and new XML() behave like the observed behavior. At the 
least for null and undefined. There needs to be a way to instantiate an empty 
XML object. I’m on the fence about objects. I’m leaning toward always throwing 
an error if the argument is an object.

Thoughts?

Harbs

[1]http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/package.html#XML()
 


> On Jul 18, 2017, at 8:20 AM, Harbs  wrote:
> 
> I’ll try to write these functions today.
> 

Re: [FlexJS] technical debt

[Harbs]

Sure. But for which events? You seem to have missed my point.

> On Jul 18, 2017, at 10:35 AM, Justin Mclean  wrote:
> 
>> How often is clone really used?
> 
> I’ve used it in just about every single application I've ever written in Flex 
> and I assume most applications will need it.

Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> I’m actually not convinced that we should enforce use of cloneEvent on all 
> events.

Again in the wiki doc I did put that it may not be required in every case.

If you have bubbles as a parameter it would be reasonable to assume that a 
cloneEvent method is needed, where bubble isn’t a parameter the intent is 
unclear but it’s probably the case you don’t want events to bubble. 6 out of 8 
issues of those have bubbles as a parameter. While the intent in not entirely 
clear in those other 2 cases it may be that they don’t need cloneEvent method.

> How often is clone really used?

I’ve used it in just about every single application I've ever written in Flex 
and I assume most applications will need it.

Thanks,
Justin

Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> How are you going to get rid of that rule if we are using different name of
> method in FlexJS ? Rule can be changed itself ?

Rule can be changed and you can create your own rules.

Given this isn't going to come up often (new events are not added that 
frequently) and it easy to see compliance. I’m not sure it's worth the effort 
of creating a new rule.

Thanks,
Justin

Re: [FlexJS] technical debt

[Harbs]

I’m actually not convinced that we should enforce use of cloneEvent on all 
events.

How often is clone really used? I just checked my app and with the exception of 
ValueEvent and composition events, every use of cloneEvent was my own events.

If cloneEvent is not used, it’s code being carried around for no reason.

Thanks,
Harbs

> On Jul 18, 2017, at 9:09 AM, piotrz  wrote:
> 
> Hi Justin,
> 
> Do you mean by this one 
> 
> "It looks to me that FielEventError, FileEvent, ContextMenuEvent and Focus
> event are missing close methods and probably require them" 
> 
> that those events are missing their own cloneEvent ? 
> 
> Thanks,
> Piotr
> 
> 
> 
> -
> Apache Flex PMC
> piotrzarzyck...@gmail.com
> --
> View this message in context: 
> http://apache-flex-development.247.n4.nabble.com/FlexJS-technical-debt-tp62851p63382.html
> Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: [FlexJS] technical debt

[piotrz]

Just didn't understand you. Thanks.

How are you going to get rid of that rule if we are using different name of
method in FlexJS ? Rule can be changed itself ?

Piotr



-
Apache Flex PMC
piotrzarzyck...@gmail.com
--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/FlexJS-technical-debt-tp62851p63385.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> "It looks to me that FielEventError, FileEvent, ContextMenuEvent and Focus
> event are missing close methods and probably require them" 
> 
> that those events are missing their own cloneEvent ? 

Yes see [1]

Thanks,
Justin

1. 
https://builds.apache.org/analysis/component_issues?id=org.apache.flex.flexjs.framework%3Aflexjs-framework-parent#resolved=false|types=BUG

Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> I think it was reasonable to switch off rule [1]. That's something which we
> shouldn't be forced to do by the sonar.

Sonar is not forcing anyone to do anything. You’ll note at the top of the 
document I put:

Rule violations:
• that are critical or blocker generally indicate bugs and should be 
fixed as soon possible.
• that are major may point to areas where issues can occur and care 
should be taken. You may want to fix these.
• that are minor can be generally ignored but you may want to comply 
with them when writing new code.

So only critical / blocker issue are things you need to look into. That rule 
isn’t a critical or blocking issue.

Thanks,
Justin

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[Justin Mclean]

Hi,

> No. The consensus was that the compiler needs improvement to replace static 
> constants with string literals and for now to not enforce one way or the 
> other.

I’m not sure how you get that from the two threads he had on this. I’m happy to 
go through and summaries the thread for you if you want or if you prefer let 
call a VOTE and abide by the results of that?

> Until the compiler is fixed, my personal preference remains to use string 
> literals.

You personal preference that may be that but you changed code so that it not 
longer uses constants.

>> I also note the code is using "”+requestStatus to convert a number to a 
>> string. Any reason for not using the toString or String(requestStatus) 
>> instead?
> 
> It’s more concise.

It’s also buggy (for large numbers for instance) so I would take care in using 
it, if you were worried about null or undefined then String(requestStatus) will 
do what you need and is much clearer to understand.

Thanks,
Justin

Re: [FlexJS] technical debt

[piotrz]

Hi Justin,

Do you mean by this one 

"It looks to me that FielEventError, FileEvent, ContextMenuEvent and Focus
event are missing close methods and probably require them" 

that those events are missing their own cloneEvent ? 

Thanks,
Piotr



-
Apache Flex PMC
piotrzarzyck...@gmail.com
--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/FlexJS-technical-debt-tp62851p63382.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[piotrz]

No problem! We have your words on Dev list! :P :)

Piotr



-
Apache Flex PMC
piotrzarzyck...@gmail.com
--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-Added-support-for-JS-upload-progress-events-tp63275p63380.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: [FlexJS] technical debt

[Justin Mclean]

Hi,

> event.clone() is wrong for FlexJS. It’s event.cloneEvent()

Which if you actually looked none of those 8 examples include a cloneEvent 
method either.

It looks to me that FielEventError, FileEvent, ContextMenuEvent and Focus event 
are missing close methods and probably require them. TodoListEvent should also 
probably have one. Intent wise it not clear if ProductFilterEvent (both of 
them) need it but I'd say this code in ProductListEvent certainly needs one:

   //making the default bubbles behavior of the event to true since we want
   //it to bubble out of the ProductListItem and beyond
   public function ProductListEvent(type:String, bubbles:Boolean=true, 
cancelable:Boolean=false)
   {
   super(type, bubbles, cancelable);
   }

Note the comment, the default of bubbles to true but no cloneEvent method.

Would you not agree that some (if not all) of these need fixing?

> A lot of the rules fly in the face to current convention in the SDK.

Which ones in particular?

> Some (such as returning from a constructor) are actually enforced by the 
> compiler so the rule is not needed.

If that the case there no harm in having them on as there should be no 
violations right?

> Feel free to do what you want to SonarQube now, but don’t make any changes 
> based on the reports.

You are free to review any commits and I make and veto any them on their 
technical merits. You are not free to revert any changes without discussion or 
a veto. Everyone is free to scratch their own itch here and the tools you use 
to find any issues should be irrelevant.

Thanks,
Justin

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[Harbs]

It’s kind of low on my priority list right now… ;-)

> On Jul 18, 2017, at 8:53 AM, piotrz  wrote:
> 
> Harbs,
> 
> If there is no jira around there about that - please raise one once you get
> a chance.
> 
> Thanks,
> Piotr
> 
> 
> 
> -
> Apache Flex PMC
> piotrzarzyck...@gmail.com
> --
> View this message in context: 
> http://apache-flex-development.247.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-Added-support-for-JS-upload-progress-events-tp63275p63377.html
> Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[piotrz]

Harbs,

If there is no jira around there about that - please raise one once you get
a chance.

Thanks,
Piotr



-
Apache Flex PMC
piotrzarzyck...@gmail.com
--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/Re-git-commit-flex-asjs-refs-heads-develop-Added-support-for-JS-upload-progress-events-tp63275p63377.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: [FlexJS] technical debt

[piotrz]

Hi,

I think it was reasonable to switch off rule [1]. That's something which we
shouldn't be forced to do by the sonar.

[1] https://builds.apache.org/analysis/coding_rules#rule_key=flex%3AS1440

Piotr



-
Apache Flex PMC
piotrzarzyck...@gmail.com
--
View this message in context: 
http://apache-flex-development.247.n4.nabble.com/FlexJS-technical-debt-tp62851p63376.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.

Re: git commit: [flex-asjs] [refs/heads/develop] - Added support for JS upload progress events

[Harbs]

> On Jul 18, 2017, at 5:41 AM, Justin Mclean  wrote:
> 
> Hi,
> 
> I can see these event name changes haven’t been reverted yet. Do you want me 
> to do that for you?

No. The consensus was that the compiler needs improvement to replace static 
constants with string literals and for now to not enforce one way or the other.

Until the compiler is fixed, my personal preference remains to use string 
literals.

> 
> I also note the code is using "”+requestStatus to convert a number to a 
> string. Any reason for not using the toString or String(requestStatus) 
> instead?

It’s more concise.

> Thanks,
> Justin

Re: [FlexJS] technical debt

[Harbs]

event.clone() is wrong for FlexJS. It’s event.cloneEvent()

There was other feedback on the dev list. That seemed to have been ignored.

A lot of the rules fly in the face to current convention in the SDK. Some (such 
as returning from a constructor) are actually enforced by the compiler so the 
rule is not needed. I don’t have time right now to get involved in a long 
protracted discussion, but please don’t take the lack of input as consensus.

Feel free to do what you want to SonarQube now, but don’t make any changes 
based on the reports. If anyone else takes an interest in it, please expect 
some rules to be changed.

Thanks,
Harbs

> On Jul 18, 2017, at 8:50 AM, Justin Mclean  wrote:
> 
> Hi,
> 
> So more than a week has gone by and I’ve received little feedback on [1]. 
> Given that I’ll gone ahead and implement the rules as discussed in the 
> document so people can see the changes.
> 
> The new results are up:
> - First off the header rules is overly strict and expects a copyright line 
> not just a header. ASF policy to not to have copyright lines so this is a 
> false positive. I’ll disable the rule. We have other means to check headers. 
> This will be fixed in the next run.
> - The ASDocs rule is picking a lot of public methods and properties that 
> don’t have ASDocs. I think we should keep this to remind us at some point it 
> needs to be fixed / people can fix as they make edits.
> - Is easier to see if issues that do need to be fixed. For instance I can see 
> that there are a couple of event.clone issues that are likely bugs.
> 
> Thanks,
> Justin
> 
> 1.  https://cwiki.apache.org/confluence/display/FLEX/Sonar+Cube+Flex+Rules 
>