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

Update "Chapter 2: Using Pulsar" documentation #59

Merged
merged 11 commits into from
Nov 11, 2022

Conversation

Daeraxa
Copy link
Member

@Daeraxa Daeraxa commented Oct 1, 2022

Updating the "Using Pulsar" docs to remove Atom references etc and modify where necessary.

All the Atom Ch2 pages have now been added and should be working for our framework.
That means:

  • Most Atom references rebranded as Pulsar
  • Links to the atom repo have been replaced with pulsar-edit repo
  • Some examples changes made such as the ppm search section for the packages it returns
  • Links all changed to internal header links that work over the whole single index.md file.
  • Pictures left untouched, we still need to have a single "theme" for redoing all our pictures of the application with atom references removed.
  • Some references to apm and ATOM_HOME have been left and will be updated when these are done in core
  • Some links to other chapters will not work yet and may need refactoring when those docs are included.
  • Emojis for Linux🐧 , macOS🍎 and Windows🪟 will be replaced with OS correct FA6 icons once Child Theme and font awesome #62 is merged. as per Discord poll these are now replaced with LNX/MAC/WIN.

Other comments

  • Wording is pretty much exactly as it was in the flight manual - this PR is not about rewording the entire documentation. This is to get these docs usable ASAP so if there are comments about wording and general refactoring then please create a PR or an issue once this has been merged.
  • Comments about the "platform switching" are welcome. I've tried my best to strike a decent balance between the "tab" switcher for the different OSs (at the cost of duplicated info) which is clunky and disruptive if used too often and the "inline" key shortcuts/paths etc. (like 🐧/🪟<kbd>Ctrl+,</kbd> - 🍎<kbd>Cmd+,</kbd>) which work better when occasionally referenced in larger sections and don't cause unnecessary duplication of text.
    • These were originally handled in the flight manual by a global platform switcher and html classes which made the markdown docs messy and hard to read, we don't have a direct equivalent of this in the new docs.

This has all been tested on the main branch of this repo and works just fine in dev from what I can see but please try the links etc. in case I have missed anything or if there are any missing images due to bad references etc.

ghost
ghost previously approved these changes Oct 1, 2022
@Daeraxa
Copy link
Member Author

Daeraxa commented Oct 2, 2022

@softcode589 I don't think you should approve draft PRs, this is only a placeholder at the moment to show that I'm working on this chapter and there are a hell of a lot of mistakes and problems still in the content. Most of the links don't work and I've not even run it on dev.

@Daeraxa
Copy link
Member Author

Daeraxa commented Oct 3, 2022

The rest has been pushed, see the PR itself for the exact comments on this.

@Daeraxa
Copy link
Member Author

Daeraxa commented Nov 4, 2022

As per the discord poll I've removed the emojis and replaced with three letter abbreviations (LNX/MAC/WIN).
Package website URL has been added and TODO removed for it.

@kaosine could you resolve the outdated change request?

Daeraxa and others added 2 commits November 7, 2022 21:20
confused-Techie
confused-Techie previously approved these changes Nov 9, 2022
Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

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

This PR looks good.

It's been open for quite some time without to much activity. Unless there's any final suggestions or issues to raise, I'll go ahead and merge in a bit.

As always thanks for your contributions, this was a big one

@confused-Techie confused-Techie dismissed kaosine’s stale review November 9, 2022 03:31

This review is outdated

icecream17
icecream17 previously approved these changes Nov 9, 2022
Copy link
Contributor

@icecream17 icecream17 left a comment

Choose a reason for hiding this comment

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

the commands in basic customization are already mindblowing enough that I feel the mindblown emoji ctrl+left, right change if made would be a separate pr

@confused-Techie
Copy link
Member

I was going to initially merge this, but somehow missed this before.

The Test Deployment is failing on this PR. @Daeraxa would you be able to take a look at this?

@Daeraxa Daeraxa dismissed stale reviews from icecream17 and confused-Techie via 0abf11b November 10, 2022 13:37
@Daeraxa
Copy link
Member Author

Daeraxa commented Nov 10, 2022

There was a dodgy link that I somehow missed and also didn't see the failing test for some reason.
It should be fixed now.

Copy link
Member

@confused-Techie confused-Techie 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 fixing those issues. With my previous approval, and tests now passing lets merge!

@confused-Techie confused-Techie merged commit 991fe12 into pulsar-edit:main Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants