Skip to content
This repository was archived by the owner on Feb 20, 2019. It is now read-only.

Conversation

@menelic
Copy link
Contributor

@menelic menelic commented Aug 12, 2014

I suggest to amend the documentation and add the following to the apps.owncloud.com website as well. Since this is a new section and describes issues under active discussion I am putting this up as a partial solution to #473 inviting comments and corrections as well as instructions from the maintainers as to where exactly this might be placed in the documentation.

I suggest to amend the documentation and add the following to the apps.owncloud.com website as well. Since this is a new section and describes issues under active discussion I am putting this up as an issue, inviting comments and corrections as well as instructions from the maintainers as to where exactly this might be placed in the documentation.
@jospoortvliet
Copy link
Contributor

Thanks for doing this! I think changes belong to this page: http://doc.owncloud.org/server/7.0/admin_manual/configuration/configuration_apps.html

Perhaps you could go over that and add the text you wrote in the appropriate spot? I think a lot of that information is already there, it just needs to be linked to from the app installer in ownCloud and from apps.owncloud.org

@karlitschek you agree? If so, how do we get a link to the documentation in those places?

@menelic
Copy link
Contributor Author

menelic commented Aug 13, 2014

thanks @jospoortvliet, I had a look at the app configuration section of the admin documentation - while its true that I can add what I wrote in there, I do think that the problem is really with the way the documenation is structured: There is a "configuring apps" section, but newcomers such as myself would really need a separate section on how apps can be installed - consider the possibility that OwnCloud might be the first self-hosted project for a user. Such a person would need something thats a bit more step-by-step, where the app installation process is really described. I think having a separate section would also avoid cluttering the "configuration_apps" section with stuff that might be obvious to some, but not to everyone.

The fact that there are different categories of apps and the question of security checks is a related, but separate issue. It would be great to have some additional info on the security status of apps in the app store. Many people would see the availability in the app store as some kind of endorsement that implies that some minimal checks have taken place. I gather from discussions here on github that this is not happening (yet)? It would be great to have some info on this to add to the app installation documentation.

Linking to the documentation:

I think ownCloud UX could benefit from more internal links to the specific, relevant section of the documentation, for example the message "ownCloud 7.0.1 is available. Get more information on how to update." is a promising link - but it simply leads to owncloud.org. To be able to create better internal links, it would be good to have smaller, more concise chunks of documentation, meaning eg a separation of "app installation" and "app configuration".

@karlitschek I m ready to propose and create such links for other non-self explanatory parts of ownCloud - if that is wanted.

@jospoortvliet
Copy link
Contributor

@menelic do you think we could add a 'installation' section at the beginning of this document? If so - any chance you could make a draft for that? I will help review and edit where I can.

Copy link
Contributor

Choose a reason for hiding this comment

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

ownCloud is the correct spelling.

@vgezer
Copy link
Contributor

vgezer commented Aug 20, 2014

I did a first proofreading. Also you need to add this file into

user_manual/index.rst. Maybe below bookmarks section similar to this:

Other Apps
==========
The following topic describes how to install other ownCloud apps from
a repository:

* :doc:`installing_apps`

Copy link
Contributor

Choose a reason for hiding this comment

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

s/install/installation/

typos corrected, app is lowercase, Apps menu is uppercase in keeping with the interface conventions - or so I thought?
@menelic
Copy link
Contributor Author

menelic commented Oct 27, 2014

@jospoortvliet @Raydiation I think I implemented all (certainly most) suggestions you made above - just want to follow up on this and find out why this has not been merged yet? Is there anything that's still missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is in fact false :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry it does not link your comment to any line so I do not know which line you are referring to. If its wrong I'll remove it, but please let me know what exactly I should remove

Copy link
Contributor

Choose a reason for hiding this comment

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

++the app contains malicious code

The code checker does not check this. It is also not possible to check this ;) so just remove the line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know that this is a controversial issue and I am aware of the troubles you have with the news app, of which I'm a fan. Part of the reason I wrote this is my wrangling with the false positive the app code checker used to throw up. I agree that there are many issues with that code checker, but since users might come across info on that when they run into issues I feel some info on the checker should be in there. Maybe it could read "the app has been rejected by the internal app code checker"? We could even add a reference to your app that disables it. Alternatively, I can delete the sentence for now but think there will have to be some kind of documentation about this. Its a problem that there is so little user-facing info about these things. @jospoortvliet , what do you say?

Copy link
Contributor

Choose a reason for hiding this comment

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

? The code checker only checks for private api usage...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, but there are issues with false positives, are there not? so saying "the app has been rejected by the internal app code checker" is more correct because not every rejection is due to malicious code? As I said I can remove this for now but want to flag this as "in need of better documentation". Because non-coder end users such as myself will come across this without there being enough info in the docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are NO rejections at all because malicious code, there are only rejections because the code checker assumes usage of the private API. (which can be as well false positives)

That sentence needs to be rephrased.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I underdstand, this is why I proposed "the app has been rejected by the internal app code checker" instead? Should that go in instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

@BernhardPosselt
Copy link
Contributor

I'd remove that one line, otherwise 👍

@BernhardPosselt
Copy link
Contributor

👍

BernhardPosselt pushed a commit that referenced this pull request Oct 27, 2014
Create installing_apps.rst
@BernhardPosselt BernhardPosselt merged commit 64db762 into owncloud-archive:master Oct 27, 2014
@vgezer
Copy link
Contributor

vgezer commented Oct 28, 2014

@menelic Can you create another PR for my comment above: #474 (comment)? In this current status, the file is not in use.

Also, maybe writing the default path for app installation will be a good start for beginners. In this case, if I am a new user, I still dont have any information where to unzip the files. A short notice with

(default: <path/to/owncloud>/apps) will be more than enough.

@vgezer
Copy link
Contributor

vgezer commented Nov 21, 2014

@Raydiation @menelic @jospoortvliet This PR does not work due to my comment above. See http://doc.owncloud.org/server/7.0/user_manual/installing_apps.html

Due to that, we have a duplicate section for installing external apps: http://doc.owncloud.org/server/7.0/admin_manual/configuration/configuration_apps.html#adding-third-party-apps

@menelic
Copy link
Contributor Author

menelic commented Nov 21, 2014

@wakeup @jospoortvliet I do not understand why there is a duplicate section now, but I do have to say it's a little frustrating that this attempt to contribute to oC is hampered by what seems to be spotty communication, intransparence and resulting parallel efforts. It seems to me the sections are not duplicates, each contains information that the other one lacks, so I am not quite clear as to why @jospoortvliet and whoever else pulled the other contribution into the documentation couldn't point the authors to this previous effort.
@wakeup I don't fully understand what you advocate as the way forward now, because it seems to me the two sections on app installation would have to be combined before any additions are being made to either?
I hope this does not sound passive-aggressive, it's not - rather, it's genuine confusion as to how such contribitions are supposed to work.

@vgezer
Copy link
Contributor

vgezer commented Nov 21, 2014

@menelic I am totally agree with you, but since this document is not available in TOC, it cannot be accessed from the doc pages, therefore, maybe @jospoortvliet could not notice it. My suggestion is appending the other commit into this one and adding this to TOC. What do you think @MorrisJobke @jospoortvliet @Raydiation @DeepDiver1975?

And @menelic, recently we also had another problem similar: #471. You can check it if you want. And all PRs are appreciated, sometimes we just miss them due to tons of e-mails from github :).

@jospoortvliet
Copy link
Contributor

@carlaschroder it looks like I screwed up, missing that @menelic had already written a section on app installation. I know you plan on fixing that part - please see if his work can save you time :D

@jospoortvliet
Copy link
Contributor

@menelic apologies... really. I totally forgot this.

@carlaschroder
Copy link
Contributor

holy cow, this is a mile long. I'll see if I can decipher it.

@carlaschroder carlaschroder self-assigned this Nov 21, 2014
@carlaschroder
Copy link
Contributor

Allrightythen, this may still need a small tweak or two; the current version is http://doc.owncloud.org/server/7.0/admin_manual/installation/apps_management_installation.html. This topic belongs in the Admin manual, as users do not have app installation permissions, unless they are members of the Admin group. In which case they can read the Admin manual :) This page should cover all aspects of apps installation and management; if anything is missing anyone is welcome to open a Github issue and tag me, or submit a patch (and tag me.)

@menelic, I'm sorry this got lost in the shuffle. Ultimately the admin and user documentation are my responsiblity, so you're always welcome to ping me about any documentation issues.

(BTW, if anyone is reading this RIGHT NOW the updates have not been pushed to the Web site yet. They're doing some work on the backend so it might take awhile)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants