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

Add more stuff about debugging #661

Closed
wants to merge 4 commits into from

Conversation

liach
Copy link

@liach liach commented Feb 10, 2018

No description provided.

@Inscrutable Inscrutable added the needs review The submission is ready and needs to be reviewed label Feb 10, 2018
@ST-DDT
Copy link
Member

ST-DDT commented Feb 10, 2018

This is a good start on how to prepare for debugging, but it does not yet cover the actual debugging process.

Things I would like to see are:

  • Debugging options such as the post mixin class content output (I forgot the command line option for that)
    • Tools for decompiling the post mixing code (jd-gui or similar ones)
  • How to debug (at least some few lines , a link to tutorials for common IDEs or a link to a debug section for plugins)
    • Suggest the creation of a test plugin along with the contribution/PR.
  • Common issues you might encounter (Broken mixin, NPEs, PhaseErrors with suggestions how to fix them or a link to their respective documentation)
  • Help with finding the error in your mixin based on the stacktrace (The only help you get is the line number)

@ST-DDT
Copy link
Member

ST-DDT commented Feb 10, 2018

Addresses: #356 (Fill "Debugging Sponge from IDE" page)

@Inscrutable Inscrutable requested review from parlough and removed request for me4502 March 16, 2018 12:15
@Inscrutable Inscrutable added the input wanted We would like to hear your opinion label Mar 18, 2018
@ST-DDT
Copy link
Member

ST-DDT commented Apr 15, 2018

@liach Any updates on this?

@liach
Copy link
Author

liach commented Apr 19, 2018

Unfortunately nope. I am currently in china and cannot even log into discord (it asks me to check email while i cannot log on to my email service in china)

@Inscrutable Inscrutable added the FLARD This is gathering dust label Jun 19, 2018
@Inscrutable Inscrutable removed the request for review from parlough July 8, 2018 11:42
@Inscrutable
Copy link
Member

This is gathering dust @liach - will you be able to complete it soon?

@liach
Copy link
Author

liach commented Oct 5, 2018

Unfortunately no. I cannot find big chunks of time to devote on stuff.

@ST-DDT
Copy link
Member

ST-DDT commented Nov 16, 2018

@liach Is the current part complete enough to be merged - so that we have at least this instead of nothing?
We can always PR the remaining stuff later.

@liach
Copy link
Author

liach commented Nov 16, 2018

i'd get a developer to confirm the validity of this portion.

SpongeCommon fork. You want to test it against the corresponding SpongeForge (or the ``bleeding`` branch of it). The
procedure is as follows:

1. Clone SpongeForge repository from GitHub: Find a clean workspace, run
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if ReST will interpret this as three lists and start each one at 1.. We shall see...

Copy link
Author

Choose a reason for hiding this comment

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

I recall @Spongy builds prs automatically. Doesn't it?

Copy link
Author

Choose a reason for hiding this comment

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

Hmm. appears spongy doesn't like me (no branch 661)

@phit
Copy link
Contributor

phit commented Nov 18, 2018

that seems like a rather complicated way of doing this all, why would you have a SC branch without already having a SF or SV workspace?

what I think most here do, setup SF or SV as usual, then cd into the SpongeCommon folder and create a new branch.. really I don't see the point of anything you added here

this page if anything should simply have the sf/sv setup instructions that are also in the main repo readmes followed by what is already covered in the plugin debugging page https://docs.spongepowered.org/stable/en/plugin/debugging.html#running-the-configuration

@Spongy
Copy link

Spongy commented Nov 18, 2018

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

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.

@liach
Copy link
Author

liach commented Jan 29, 2019

Closing in favor of #784 as suggested by inscrutable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FLARD This is gathering dust input wanted We would like to hear your opinion needs review The submission is ready and needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants