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

Corrected misspellings in code comments #3577

Merged
merged 2 commits into from
Jun 22, 2015
Merged

Corrected misspellings in code comments #3577

merged 2 commits into from
Jun 22, 2015

Conversation

bkoelman
Copy link
Contributor

Manual correction of some spelling errors, one by one.
Also changed smart single/double quotes into regular quotes (except in tests that verify for their occurrence).

@bkoelman
Copy link
Contributor Author

@Pilchie Could you tag some team members for review?

@Pilchie
Copy link
Member

Pilchie commented Jun 22, 2015

@pharring @cston @VSadov

@@ -1252,7 +1252,7 @@ internal bool IsRealSigned
{
// A module cannot be signed. The native compiler allowed one to create a netmodule with an AssemblyKeyFile
// or Container attribute (or specify a key via the cmd line). When the module was linked into an assembly,
// alink would sign the assembly. So rather than give an error we just don't sign when outputting a module.
// a link would sign the assembly. So rather than give an error we just don't sign when outputting a module.
Copy link
Member

Choose a reason for hiding this comment

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

Should be 'alink' (the framework tool) I believe.

@cston
Copy link
Member

cston commented Jun 22, 2015

LGTM

@@ -323,7 +323,7 @@ private static void MarkReachableFromBranch(ArrayBuilder<BasicBlock> reachableBl
{
// if branch is blocked by a finally, then should branch to corresponding
// BlockedBranchDestination instead. Original label may not be reachable.
// if there are no blocking finallys, then BlockedBranchDestination returns null
// if there are no blocking finallies, then BlockedBranchDestination returns null
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "finally blocks" instead?

@VSadov
Copy link
Member

VSadov commented Jun 22, 2015

LGTM

VSadov added a commit that referenced this pull request Jun 22, 2015
Corrected misspellings in code comments
@VSadov VSadov merged commit 2f2629b into dotnet:master Jun 22, 2015
@bkoelman
Copy link
Contributor Author

@VSadov I would like to address review comments, but it looks like you already merged. Can I still add commits to my branch so they get added here? If not, how to proceed?

@bkoelman
Copy link
Contributor Author

@jaredpar I'm not sure how to proceed here. Can you help me?

@VSadov
Copy link
Member

VSadov commented Jun 25, 2015

one way to find out :-)
I guess you can add more commits to the branch, but you may need to open another PR

@jaredpar
Copy link
Member

@bkoelman I think the best way is to open a new PR for addressing the feedback.

@bkoelman bkoelman mentioned this pull request Jun 29, 2015
@bkoelman
Copy link
Contributor Author

Thanks for helping out guys.

I tried adding more commits to my branch. After that, Github suggested to create a new pull request, because it saw this one was already merged. Awesome!

Can someone merge #3729 for me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants