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

DOC: Death to readthedocs #667

Merged
merged 25 commits into from
Jul 12, 2023
Merged

DOC: Death to readthedocs #667

merged 25 commits into from
Jul 12, 2023

Conversation

hydroflame
Copy link
Collaborator

@hydroflame hydroflame commented Jul 10, 2023

  • Implement markdown with MUI.
  • Move and convert all rst documentation to md
  • Rework documentation a little bit.
  • Rename Tutorial tab to Documentation tab
    image

@github-actions github-actions bot added the error: invalid title PR title must follow specific format label Jul 10, 2023
@hydroflame hydroflame changed the title Remove readthedoc completely and move everything in-game. DOCUMENTATION: Remove readthedoc completely and move everything in-game. Jul 10, 2023
@github-actions github-actions bot removed the error: invalid title PR title must follow specific format label Jul 10, 2023
@hydroflame hydroflame changed the title DOCUMENTATION: Remove readthedoc completely and move everything in-game. DOC: Death to readthedocs Jul 10, 2023
@bitburner-official bitburner-official deleted a comment from github-actions bot Jul 10, 2023
Copy link
Contributor

@TheMas3212 TheMas3212 left a comment

Choose a reason for hiding this comment

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

looks good to me

src/Tutorial/ui/root.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@d0sboots d0sboots left a comment

Choose a reason for hiding this comment

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

Code generally looks good. I didn't try running it, though.

import { getPage } from "./root";
import { Navigator, useHistory } from "../../ui/React/Documentation";

const resolveRelativePath = (folder: string, relative: string): string => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pretty sure we already have utility functions for these.

Copy link
Collaborator

@Snarling Snarling Jul 11, 2023

Choose a reason for hiding this comment

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

We do but there is a little finessing needed to get the existing resolveFilePath to work here, since it defaults to using an absolute path unless the path actually starts with ./ or ../

I posted a suggestion but this can easily be dealt with later once everything is working correctly, even in a followup PR.

@@ -0,0 +1,3 @@
# Bladeburners

PLACEHOLDER
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a lot of files like this still. This is a lot of docs to bite off in one chunk. IMO it would make more sense to delete each RST as it is converted, and do them in batches, but if you want to do them all in one go I'm not gonna stop you.

src/Tutorial/ui/doc/notfound.md Outdated Show resolved Hide resolved
src/ui/MD/MD.tsx Show resolved Hide resolved
src/ui/MD/md_test.ts Outdated Show resolved Hide resolved
src/ui/MD/a.tsx Outdated Show resolved Hide resolved
Snarling

This comment was marked as resolved.

Snarling

This comment was marked as resolved.

Just committing a minimal fix so there's a working base for anyone doing testing
Build: fixed filename on an import
Lint: Removed empty function on useEffect, then removed unused useEffect import
Format: Removed a double-newline in autogen file
@Snarling
Copy link
Collaborator

@hydroflame I see what you mean with renaming the directory not working right, it works fine locally but the rename doesn't actually get reflected in the commit, so the committed version still fails to build.

I'm renaming that folder to Markdown, that way the change is not strictly capitalization and it should go through. Sorry for any inconvenience, not trying to step on your toes just trying to get this in a state where people can continue to test it without needing to apply local fixes to make the game build.

Copy link
Collaborator

@Snarling Snarling left a comment

Choose a reason for hiding this comment

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

Button links don't get the right font right now:

image

Locally I was able to fix this by getting rid of all the useStyles/makeStyles stuff in a.tsx and adding to props: variant="body1" fontSize="inherit":

image

@hydroflame
Copy link
Collaborator Author

I think addressed everything.

@Snarling No need to feel bad for fixing my stuff. I appreciate it. I didn't know it prevented other people from building though.

Now I'm going to fill in all the placeholders.

@hydroflame
Copy link
Collaborator Author

I had to rename the folder Markdown to MD again because prettier would ignore the files under it.

@hydroflame
Copy link
Collaborator Author

There are still some placeholder files but it's too mind numbing to write these things. I hope a user will get annoyed enough to write it for us.

@hydroflame
Copy link
Collaborator Author

Fix #624

@Snarling Snarling merged commit 1a8b9a9 into dev Jul 12, 2023
5 checks passed
@Snarling Snarling deleted the markdown branch July 12, 2023 21:10
@d0sboots
Copy link
Collaborator

d0sboots commented Jul 12, 2023

There are still some placeholder files but it's too mind numbing to write these things. I hope a user will get annoyed enough to write it for us.

Noooooo why not md ;_;

Edit: Replied to the wrong comment, but this comment is important too. This is exactly what I was worried about when you started by deleting all the .rst. Writing documentation is mind-numbing; you can't count on anyone else doing it. I don't think we should be deleting the unconverted docs until they have converted replacements.

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