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

[ci skip] Improve doc for ModuleConstMissing.guess_for_anonymous #15278

Merged
merged 1 commit into from
May 24, 2014

Conversation

akshay-vishnoi
Copy link
Contributor

No description provided.

@fxn
Copy link
Member

fxn commented May 23, 2014

@pixeltrix what do you think as a native English speaker?

@zzak
Copy link
Member

zzak commented May 23, 2014

Looks good to me, the grammar is definitely better.

# (unless it can be proven that is not the case), and the path to the file
# that defines the constant. Anonymous modules cannot follow these
# conventions and therefore we assume that the user wants to refer to a
# top-level constant.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd change 'Dependencies assume' to 'Dependencies assumes' since it's really a singular noun and not a plural - if you replace Dependencies with Xavier then assume sounds wrong. I'd also lose the comma after the closing parenthesis. Finally there's a change from third person to first person with the 'we' in the final sentence - either change Dependencies to We or change 'we assume' to 'it assumes'.

@akshay-vishnoi
Copy link
Contributor Author

cc: @fxn, @pixeltrix
Please have a look at it now.

pixeltrix added a commit that referenced this pull request May 24, 2014
[ci skip] Improve doc for ModuleConstMissing.guess_for_anonymous
@pixeltrix pixeltrix merged commit b850c4c into rails:master May 24, 2014
@pixeltrix
Copy link
Contributor

@akshay-vishnoi thanks!

@akshay-vishnoi
Copy link
Contributor Author

💚

@fxn
Copy link
Member

fxn commented May 24, 2014

❤️

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

Successfully merging this pull request may close these issues.

None yet

5 participants