Skip to content

Add a couple of interface definitions#4928

Merged
tanghus merged 9 commits into
appframework-masterfrom
interfaces
Sep 23, 2013
Merged

Add a couple of interface definitions#4928
tanghus merged 9 commits into
appframework-masterfrom
interfaces

Conversation

@bartv2

@bartv2 bartv2 commented Sep 20, 2013

Copy link
Copy Markdown
Contributor

@DeepDiver1975 DeepDiver1975 mentioned this pull request Sep 20, 2013
27 tasks
Comment thread lib/public/iconfig.php

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hmmm - if there is a getSystemValue() why not a setSystemValue()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do we want apps to write into config.php? is not really the place to store app values.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hmm - where did my comments go?

Well accessing config.php has some sec issues as well like getting access to the password salt, database credentials ...

@ghost

ghost commented Sep 20, 2013

Copy link
Copy Markdown

Test passed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/1128/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no return?
no exceptions?

Comment thread lib/server.php

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

return OCP please

@ghost

ghost commented Sep 20, 2013

Copy link
Copy Markdown

Test passed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/1131/

@ghost

ghost commented Sep 20, 2013

Copy link
Copy Markdown

Test passed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/1132/

@ghost

ghost commented Sep 20, 2013

Copy link
Copy Markdown

Test passed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/1135/

@ghost

ghost commented Sep 20, 2013

Copy link
Copy Markdown

Test passed.
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/1136/

@tanghus

tanghus commented Sep 23, 2013

Copy link
Copy Markdown
Contributor

@DeepDiver1975 Aren't the issues addressed now? Otherwise I think you should make separate PRs for the different cases @bartv2. We need to get this puppy merged :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

❓ 😉

@DeepDiver1975

Copy link
Copy Markdown
Member

👍 here we go

tanghus added a commit that referenced this pull request Sep 23, 2013
Add a couple of interface definitions
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants