Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup vs ext #349

Closed
wants to merge 9 commits into from
Closed

Cleanup vs ext #349

wants to merge 9 commits into from

Conversation

enricosada
Copy link
Contributor

some cleanup.

i removed the /*internal, but public for FSharp.Project.dll*/ because is annoying while reading the code

the other are style fix or cleanup

parent.onChildAdded(parent, args);
}

var foo = this.projectMgr ?? this;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are cleaning up this code, can we have a proper variable name rather than foo. We might as well clean it up fully rather than partially.

@KevinRansom
Copy link
Member

I guess if you can take care of the many repetitions of each comment, I am good with accepting this, it certainly makes the code at this part easier on the eyes. Thank you for taking care of this.

@msftclas
Copy link

@enricosada, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@KevinRansom
Copy link
Member

Thank you for taking the time to take care of this work, we will take care of this at the start of the next cycle.

Moving this to V.Next per: #371

latkin pushed a commit that referenced this pull request Aug 4, 2015
remove all of the `/*internal, but public for FSharp.Project.dll*/` comments, and other small cleanups

closes #349

commit 4a8abefeddc58809f7429434649e2f5b02f67ff7
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 16:44:03 2015 +0200

    fix naming

commit 2767f161028cfc78078eb7ac197f36bfdaed6e2e
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 16:39:48 2015 +0200

    unused comment and using

commit 784d73e80827ccb79485b07c481043842a1511a1
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 16:32:46 2015 +0200

    cleanup

commit 3d9dd20bda3f27b35ab38c342f347b3d48fdd0b7
Author: enricosada <enrico@sada.io>
Date:   Wed Apr 8 17:18:46 2015 +0200

    fix xml docs

commit c737d7c82621eef223cc894be9c6fc415ddbf67e
Author: enricosada <enrico@sada.io>
Date:   Wed Apr 8 17:16:19 2015 +0200

    remove commented out code

commit 9ddf4c15b2fa9ea1d82dffa37ae632062b24c33d
Author: enricosada <enrico@sada.io>
Date:   Wed Apr 8 17:11:35 2015 +0200

    style fix

commit 24cdafd72721d2f81711d15d7f412b1f8810d719
Author: enricosada <enrico@sada.io>
Date:   Wed Apr 8 17:10:44 2015 +0200

    possible null ref, check parent before parent.onChildAdded

commit 3c74d24fd7e9ec78ae2cfddcd9d993d74e080742
Author: enricosada <enrico@sada.io>
Date:   Wed Apr 8 17:08:11 2015 +0200

    remove unused

commit 54bdb0530c64a959c345cfffa5566f4ddba753d8
Author: enricosada <enrico@sada.io>
Date:   Wed Apr 8 17:00:50 2015 +0200

    remove comment /*[access-modifier], but [access-modifier] for FSharp.Project.dll*/
@latkin
Copy link
Contributor

latkin commented Aug 4, 2015

Applied to the OOB branch.

@latkin latkin closed this Aug 4, 2015
@latkin latkin added fixed and removed V.Next labels Aug 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants