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

more debugging clarifications and rewrites #753

Merged
merged 3 commits into from Sep 7, 2018

Conversation

phit
Copy link
Contributor

@phit phit commented Aug 24, 2018

adds to #745

@phit phit requested a review from ST-DDT August 24, 2018 21:56
@Spongy
Copy link

Spongy commented Aug 24, 2018

A preview for this pull request is available at https://cdn.rawgit.com/Spongy/SpongeDocs-PRs/8b7bd5e/.

Here are some links to the pages that were modified:

Since the preview frequently changes, please link to this comment, not to the direct url to the preview.

@phit phit added input wanted We would like to hear your opinion needs review The submission is ready and needs to be reviewed low priority Small issues like typos that don't have much of an impact labels Aug 24, 2018
Copy link
Member

@Inscrutable Inscrutable left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up. It was looking Nasty. ;)
I have cleaned up the grammar.

Sponge bug (or another coremod being present).
* Other classes have to be mapped to their mods by hand. In this case there is this entry
``com.example.extendedaiplugin``, Java projects usually follow the Maven package naming standard
Copy link
Member

Choose a reason for hiding this comment

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

comma after com.example.extendedaiplugin should be a semicolon.
comma needed after groupId.groupId.artifactId
semicolon needed after "backwards"
comma needed after example.com
comma or hyphen needed after "name of the project" (or put "also known as artifact in the Java world" in brackets)
comma after ``extendedaiplugin` should be a semicolon
comma needed after "when in doubt"


.. warning::
Be careful when coremods are present, it can mean that although a Minecraft class was reported as the cause, it does
Copy link
Member

Choose a reason for hiding this comment

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

comma after "coremods are present" should be a semicolon (or end the sentence there and start a new one at "It ...").
comma needed after "it can mean that"
comma (or hyphen) needed after "Minecraft source"
Remove "Though" and start final sentence with "Be"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't like commas 😆
ill fix it when i get up in the morning

Copy link
Member

Choose a reason for hiding this comment

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

Don't go into a comma coma 😴

Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

I'm not sure (because the headlines are not real headlines), but headlines should use Title Case.

Java projects usually follow the Maven package naming standard

I'm not sure that this is correct, but using the web search is a good hint/alternative.

All in all its a good improvement.

@Inscrutable
Copy link
Member

I missed the headlines/titles bit, they used to be more like bullet points. They are all sentences anyway. We might let this slide, or we could revert them to "(1) blah" style (note parentheses) or just individual bullets.

@phit
Copy link
Contributor Author

phit commented Aug 27, 2018

I feel like the page looked really odd before with how the checklist was presented, if we don't make them small headings I would like to at least make them bold.

@ST-DDT
Copy link
Member

ST-DDT commented Sep 1, 2018

Please resolve merge conflicts.

@Inscrutable Inscrutable removed the needs review The submission is ready and needs to be reviewed label Sep 7, 2018
@Inscrutable Inscrutable merged commit 2016495 into stable Sep 7, 2018
@phit phit deleted the more-debugging-improvements branch September 7, 2018 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
input wanted We would like to hear your opinion low priority Small issues like typos that don't have much of an impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants