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 documentation for the Outline pane #327

Merged
merged 13 commits into from
Oct 2, 2022

Conversation

hyounes4560
Copy link
Contributor

Pull Request

Pull Request Checklist

  • Read and followed this repo's Contributing Guidelines
  • Based your PR on the latest version of the correct branch (master or 4.x)
  • Checked your writing carefully for correct English spelling, grammar, etc
  • Described your changes and the motivation for them below
  • Noted what issue(s) this pull request resolves, creating one if needed

Description of Changes

Add a new section for the Outline pane as per @CAM-Gerlach suggestion.

Issue(s) Resolved

Fixes #326

@hyounes4560 hyounes4560 changed the title Add documentation for the outline pane #326 Add documentation for the outline pane Sep 21, 2022
Copy link
Member

@steff456 steff456 left a comment

Choose a reason for hiding this comment

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

Left a minor comment but overall looks good! Thanks for working on this :)

doc/panes/outline.rst Outdated Show resolved Hide resolved
@CAM-Gerlach
Copy link
Member

Thanks @hyounes4560 for all your hard work on this! I apologize for my absence over the past week; my mom's living situation became unstable again and I had to go help her move (again).

Per your request, I went ahead and took care of the minor changes as commits on my side. The changes are:

  • Tweaking the crop on the options menu image to omit the area below, which was taking up a good deal of vertical space, and compressing the resulting image
  • Cleaning up line breaks and spaces
  • Fixing a couple issues with the reST formatting (particularly GDocs' mangling of the :menuselection: syntax) and applying reST formatting a few other places
  • Using consistent capitalization (Outline to refer to the pane and outline to the actual outline construct) and verb tense in the Options menu list

I broke them up into separate commits for your review, but they'll all be squashed into your commit when this is merged. Thanks!

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Thanks again @hyounes4560 , this looks good to me! I have two totally optional suggestions you could consider, but they could, at your discretion, be left for another PR or skipped entirely:

  • You might want to move the options menu image to above the list of options, between "The options menu in the..." and "These customization settings include:", so users have a visual overview of the options before viewing them in detail, like you did with the Components section, and because it seems to make more sense that way and fit with the others.
  • You could add a Related panes section at the bottom like the other panes have, which could perhaps list the Editor, Files pane and Project pane.

Besides your and @steff456 's approval, I'll also leave this open for at least a few days for others (e.g. @ccordoba12 , @jitseniesen , etc) who might have helpful feedback. Thanks!

@CAM-Gerlach CAM-Gerlach changed the title Add documentation for the outline pane Add documentation for the Outline pane Sep 23, 2022
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @hyounes4560 for your work on this! A couple of small suggestions for you, otherwise looks good to me.

doc/panes/outline.rst Outdated Show resolved Hide resolved
doc/panes/outline.rst Outdated Show resolved Hide resolved
@jitseniesen
Copy link
Member

Looks good; well done, Hanan! No further comments from me.

hyounes4560 and others added 4 commits September 26, 2022 18:10
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
@CAM-Gerlach CAM-Gerlach dismissed steff456’s stale review September 26, 2022 23:38

Requested change was applied

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Just a few small changes to clean up a couple minor issues due to the local addition and subsequent merge (due to not having pulled the updated branch first), otherwise LGTM, thanks

doc/panes/outline.rst Outdated Show resolved Hide resolved
doc/panes/outline.rst Outdated Show resolved Hide resolved
doc/panes/outline.rst Outdated Show resolved Hide resolved
@CAM-Gerlach
Copy link
Member

Pro tip—I find it a good habit to always git pull before I make any local changes to pick up any remote ones you or others pushed the the PR, as it avoids the merge conflict and related issues you ran into here (as otherwise, if I don't make it a habit, I find it easy to forget).

Also, quick reminder, you can apply multiple suggestions at time with the same message by (on the Filestab) clicking Add to batch on each suggestion and then Commit once you'd added all of them.

So long as we're squash merging, neither of these issues poses major problems at our end—but it can certainly help things go smoother on yours :)

hyounes4560 and others added 3 commits October 2, 2022 09:14
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @hyounes4560 !

@CAM-Gerlach CAM-Gerlach merged commit 8b8c327 into spyder-ide:master Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the Outline pane docs
5 participants