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

Creating Help Articles folder #123

Merged
merged 1 commit into from Oct 11, 2022
Merged

Conversation

jpwalden
Copy link
Member

Signed-off-by: jari-pekka walden jari-pekka.walden@jolla.com

@CLAassistant
Copy link

CLAassistant commented Sep 27, 2022

CLA assistant check
All committers have signed the CLA.

Support/Help_Articles/README.md Outdated Show resolved Hide resolved
@vigejolla
Copy link
Member

I don't fully see how this PR maps to the task 58506, are you sure the bug number in the branch name is correct?

On a related note, I don't think it makes sense to merge this PR without any "subpages". So, if your intention was indeed to add the page mentioned in task 58506 here, I suggest you add it in this same PR.

@jpwalden
Copy link
Member Author

I don't fully see how this PR maps to the task 58506, are you sure the bug number in the branch name is correct?

On a related note, I don't think it makes sense to merge this PR without any "subpages". So, if your intention was indeed to add the page mentioned in task 58506 here, I suggest you add it in this same PR.

Yes, the intention was to add that ZD article, creating of the folder is the 1st step, therefore we used that ID.

@vigejolla
Copy link
Member

Please squash the commits

Copy link
Member

@vigejolla vigejolla left a comment

Choose a reason for hiding this comment

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

LGTM, but please don't merge this before you have some content for the folder as well

@jovirkku
Copy link
Contributor

The created folder Support/Help_Articles will contain plenty of subfolders, each for one help article.

@jpwalden jpwalden force-pushed the jb58506 branch 3 times, most recently from ca065ff to edcaaf3 Compare September 30, 2022 12:40
Copy link
Contributor

@jovirkku jovirkku left a comment

Choose a reason for hiding this comment

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

Line 142 in README.md: the code has "Sailfish-dir-content-3.1.0.11.PNG" while the picture has the extension .png ==> the picture is not shown in chapter 4 (picture 4b).

Line 104 has "...with Android O****S."

Chapter 4: "...is always available in this repository []"
As that sentence has the hyperlink then perhaps the "[
]" and the later (two lines down) URL are not necessary.

The header numbers, then. I think this DOC service has an automatic TOC and numbering of headers. The manually written numbers might lead to duplicate numbering. Not sure, though.

Otherwise, it looks good.

@jpwalden jpwalden requested a review from rainemak October 3, 2022 09:17
Copy link
Member

@vigejolla vigejolla left a comment

Choose a reason for hiding this comment

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

Please fix also those findings from @jovirkku

Support/Help_Articles/Installing_Sailfish/README.md Outdated Show resolved Hide resolved
Support/Help_Articles/Installing_Sailfish/README.md Outdated Show resolved Hide resolved
Signed-off-by: jari-pekka walden <jari-pekka.walden@jolla.com>
Copy link
Member

@vigejolla vigejolla left a comment

Choose a reason for hiding this comment

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

LGTM

@vigejolla vigejolla merged commit 41dbe10 into sailfishos:master Oct 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants