You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by SlavaRa <gi...@git.apache.org> on 2015/10/17 09:12:05 UTC

[GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

GitHub user SlavaRa opened a pull request:

    https://github.com/apache/flex-sdk/pull/22

    [Module asc] Cleanup code - 1st step

    This PR is the first step on the path to good condition code of compiler.
    I hope that you will take it and then I can make the new PR.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/SlavaRa/flex-sdk cleanup_asc_code

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flex-sdk/pull/22.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #22
    
----
commit 68b3ab2d9e6a902b6e1e286f1c011e8c5c30280f
Author: SlavaRa <ra...@ya.ru>
Date:   2015-10-17T06:54:27Z

    [Module asc] Cleanup code - 1st step

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

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

> Should we encourage SlavaRa to spend his energy on Falcon, or take his
> pull requests and hope they are good?

"Scratch your own itch” as they say. I don’t think we should be discouraging any contribution, the old compiler is still in use so we should at least consider them.

>  Does anybody else have the cycles to review the PR?

I’ll take a look in the next couple of days unless someone beats me to it.

Thanks,
Justin

Re: [GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

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

On 10/20/15, 11:43 PM, "Harbs" <ha...@gmail.com> wrote:

>He just closed the pull request. I’m not sure why.

Hmm.  He just created new pull requests.  Well, what do others think?
Should we encourage SlavaRa to spend his energy on Falcon, or take his
pull requests and hope they are good?  Does anybody else have the cycles
to review the PR?

-Alex


Re: [GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

Posted by Harbs <ha...@gmail.com>.
He just closed the pull request. I’m not sure why.

On Oct 19, 2015, at 10:48 PM, OmPrakash Muppirala <bi...@gmail.com> wrote:

> You could just comment on the PR at GitHub.  The messages and responses get
> forwarded to the dev list.
> 
> Thanks,
> Om
> 
> On Mon, Oct 19, 2015 at 10:45 AM, Alex Harui <ah...@adobe.com> wrote:
> 
>> 
>> 
>> On 10/19/15, 9:03 AM, "omuppi1@gmail.com on behalf of OmPrakash Muppirala"
>> <omuppi1@gmail.com on behalf of bigosmallm@gmail.com> wrote:
>> 
>>> Alex, et al.,
>>> 
>>> Can you please take a look at this PR  and respond accordingly?
>> 
>> Well, I looked at a few screenfuls of the diffs when it first came in.  It
>> appears to be an actual code clean up a portion of the MXMLC compiler.  I
>> saw things like “import java.util.*” being replaced by the actual classes
>> that are needed, and “someString.indexOf('foo') > -1” being replaced by
>> “someString.contains('foo’)"
>> 
>> I have to admit that I am not motivated to scrub this patch looking for
>> potential errors.  I would be much more interested if the clean up was in
>> the Falcon code base.  While I want to encourage all kinds of
>> contributions and recruit more committers, I don’t see how this patch
>> “moves the needle”.  Adding to that, I don’t see the email address of the
>> PR author on our dev@ subscriber list, and it seems we should engage the
>> author in a dialog about “why” this PR was generated and/or try to
>> redirect his/her efforts to the Falcon code base.
>> 
>> If some other committer has the motivation to review the PR, please do so.
>> IMO, you don’t have to know anything about the compiler and only some
>> basics about Java.  And we could also gamble by just having someone commit
>> the PR and see what breaks if anything, but there is always the risk of
>> subtle errors being introduced.  I wouldn’t worry about that at all for
>> Falcon’s code base since the whole thing is in flux, but it would not be
>> good to de-stabilize MXMLC.
>> 
>> Meanwhile, it seems like we should contact the author and encourage them
>> to discuss the PR on our dev@ list.
>> 
>> Thoughts?
>> -Alex
>> 
>> 
>> 


Re: [GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

Posted by OmPrakash Muppirala <bi...@gmail.com>.
You could just comment on the PR at GitHub.  The messages and responses get
forwarded to the dev list.

Thanks,
Om

On Mon, Oct 19, 2015 at 10:45 AM, Alex Harui <ah...@adobe.com> wrote:

>
>
> On 10/19/15, 9:03 AM, "omuppi1@gmail.com on behalf of OmPrakash Muppirala"
> <omuppi1@gmail.com on behalf of bigosmallm@gmail.com> wrote:
>
> >Alex, et al.,
> >
> >Can you please take a look at this PR  and respond accordingly?
>
> Well, I looked at a few screenfuls of the diffs when it first came in.  It
> appears to be an actual code clean up a portion of the MXMLC compiler.  I
> saw things like “import java.util.*” being replaced by the actual classes
> that are needed, and “someString.indexOf('foo') > -1” being replaced by
> “someString.contains('foo’)"
>
> I have to admit that I am not motivated to scrub this patch looking for
> potential errors.  I would be much more interested if the clean up was in
> the Falcon code base.  While I want to encourage all kinds of
> contributions and recruit more committers, I don’t see how this patch
> “moves the needle”.  Adding to that, I don’t see the email address of the
> PR author on our dev@ subscriber list, and it seems we should engage the
> author in a dialog about “why” this PR was generated and/or try to
> redirect his/her efforts to the Falcon code base.
>
> If some other committer has the motivation to review the PR, please do so.
>  IMO, you don’t have to know anything about the compiler and only some
> basics about Java.  And we could also gamble by just having someone commit
> the PR and see what breaks if anything, but there is always the risk of
> subtle errors being introduced.  I wouldn’t worry about that at all for
> Falcon’s code base since the whole thing is in flux, but it would not be
> good to de-stabilize MXMLC.
>
> Meanwhile, it seems like we should contact the author and encourage them
> to discuss the PR on our dev@ list.
>
> Thoughts?
> -Alex
>
>
>

Re: [GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

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

On 10/19/15, 9:03 AM, "omuppi1@gmail.com on behalf of OmPrakash Muppirala"
<omuppi1@gmail.com on behalf of bigosmallm@gmail.com> wrote:

>Alex, et al.,
>
>Can you please take a look at this PR  and respond accordingly?

Well, I looked at a few screenfuls of the diffs when it first came in.  It
appears to be an actual code clean up a portion of the MXMLC compiler.  I
saw things like “import java.util.*” being replaced by the actual classes
that are needed, and “someString.indexOf('foo') > -1” being replaced by
“someString.contains('foo’)"

I have to admit that I am not motivated to scrub this patch looking for
potential errors.  I would be much more interested if the clean up was in
the Falcon code base.  While I want to encourage all kinds of
contributions and recruit more committers, I don’t see how this patch
“moves the needle”.  Adding to that, I don’t see the email address of the
PR author on our dev@ subscriber list, and it seems we should engage the
author in a dialog about “why” this PR was generated and/or try to
redirect his/her efforts to the Falcon code base.

If some other committer has the motivation to review the PR, please do so.
 IMO, you don’t have to know anything about the compiler and only some
basics about Java.  And we could also gamble by just having someone commit
the PR and see what breaks if anything, but there is always the risk of
subtle errors being introduced.  I wouldn’t worry about that at all for
Falcon’s code base since the whole thing is in flux, but it would not be
good to de-stabilize MXMLC.

Meanwhile, it seems like we should contact the author and encourage them
to discuss the PR on our dev@ list.

Thoughts?
-Alex



Re: [GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

Posted by OmPrakash Muppirala <bi...@gmail.com>.
Alex, et al.,

Can you please take a look at this PR  and respond accordingly?

Thanks,
Om
On Oct 17, 2015 12:12 AM, "SlavaRa" <gi...@git.apache.org> wrote:

> GitHub user SlavaRa opened a pull request:
>
>     https://github.com/apache/flex-sdk/pull/22
>
>     [Module asc] Cleanup code - 1st step
>
>     This PR is the first step on the path to good condition code of
> compiler.
>     I hope that you will take it and then I can make the new PR.
>
> You can merge this pull request into a Git repository by running:
>
>     $ git pull https://github.com/SlavaRa/flex-sdk cleanup_asc_code
>
> Alternatively you can review and apply these changes as the patch at:
>
>     https://github.com/apache/flex-sdk/pull/22.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
>     This closes #22
>
> ----
> commit 68b3ab2d9e6a902b6e1e286f1c011e8c5c30280f
> Author: SlavaRa <ra...@ya.ru>
> Date:   2015-10-17T06:54:27Z
>
>     [Module asc] Cleanup code - 1st step
>
> ----
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>

[GitHub] flex-sdk pull request: [Module asc] Cleanup code - 1st step

Posted by SlavaRa <gi...@git.apache.org>.
Github user SlavaRa closed the pull request at:

    https://github.com/apache/flex-sdk/pull/22


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---