Skip to content

Document phoenix.baseUrl in config.sample.php#36007

Merged
DeepDiver1975 merged 1 commit intomasterfrom
bugfix/add-phoenix-base-ur-to-config.sample
Aug 16, 2019
Merged

Document phoenix.baseUrl in config.sample.php#36007
DeepDiver1975 merged 1 commit intomasterfrom
bugfix/add-phoenix-base-ur-to-config.sample

Conversation

@DeepDiver1975
Copy link
Copy Markdown
Member

@DeepDiver1975 DeepDiver1975 commented Aug 9, 2019 •
edited
Loading

Description

The new config parameter phoenix.baseUrl is now documented in config.sample.php

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@DeepDiver1975 DeepDiver1975 requested a review from settermjd August 9, 2019 10:02
@DeepDiver1975 DeepDiver1975 self-assigned this Aug 9, 2019
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 9, 2019

Codecov Report

Merging #36007 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #36007   +/-   ##
=======================================
  Coverage   53.85%   53.85%           
=======================================
  Files          63       63           
  Lines        7377     7377           
  Branches     1301     1301           
=======================================
  Hits         3973     3973           
  Misses       3019     3019           
  Partials      385      385
Flag Coverage Δ
#javascript 53.85% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cefd5e...0158c31. Read the comment docs.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 9, 2019 •
edited
Loading

Codecov Report

Merging #36007 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #36007   +/-   ##
=======================================
  Coverage   53.85%   53.85%           
=======================================
  Files          63       63           
  Lines        7377     7377           
  Branches     1301     1301           
=======================================
  Hits         3973     3973           
  Misses       3019     3019           
  Partials      385      385
Flag Coverage Δ
#javascript 53.85% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2908c40...ba815fa. Read the comment docs.

@micbar micbar added this to the development milestone Aug 9, 2019
mmattel
mmattel reviewed Aug 12, 2019
View reviewed changes
Comment thread config/config.sample.php

/**
* If phoenix.baseUrl is set public and private links will be redirected to this
* url. Phoenix will handle these links accordingly.
Copy link
Copy Markdown
Contributor

@mmattel mmattel Aug 12, 2019

Choose a reason for hiding this comment

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

But only if the Phoenix App is loaded and enabled...
(Additional, add a comma for better readability: is set public --> is set, public)
Maybe to tell why this is necessary ? A brief reason would help.

Copy link
Copy Markdown
Member Author

@DeepDiver1975 DeepDiver1975 Aug 12, 2019

Choose a reason for hiding this comment

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

But only if the Phoenix App is loaded and enabled...

Phoenix can operate independent from owncloud - there is no need in having it installed as an app

Copy link
Copy Markdown
Contributor

@mmattel mmattel Aug 12, 2019

Choose a reason for hiding this comment

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

confusing, as there is an app on the marketplace...

Copy link
Copy Markdown
Member Author

@DeepDiver1975 DeepDiver1975 Aug 13, 2019

Choose a reason for hiding this comment

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

To be precise:

  • phoenix can be installed as an owncloud app
  • phoenix can be installed on any web space (basically when installed as an app nothing else then serving the web assets is done by owncloud/apache)

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/add-phoenix-base-ur-to-config.sample branch from 0158c31 to c101868 Compare August 14, 2019 08:06
settermjd
settermjd reviewed Aug 14, 2019
View reviewed changes
Comment thread config/config.sample.php Outdated
* If phoenix.baseUrl is set, public and private links will be redirected to this
* url. Phoenix will handle these links accordingly.
*
* As an example, in case 'phoenix.baseUrl' is set to 'http://phoenix,example.com,
Copy link
Copy Markdown
Contributor

@settermjd settermjd Aug 14, 2019

Choose a reason for hiding this comment

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

The URL needs a trailing single quote.

settermjd
settermjd suggested changes Aug 14, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@settermjd settermjd left a comment

Choose a reason for hiding this comment

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

Other than the missing, trailing, single quote, it looks good to me.

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/add-phoenix-base-ur-to-config.sample branch from c101868 to ba815fa Compare August 14, 2019 11:43
@DeepDiver1975 DeepDiver1975 merged commit 9606d8f into master Aug 16, 2019
@delete-merged-branch delete-merged-branch Bot deleted the bugfix/add-phoenix-base-ur-to-config.sample branch August 16, 2019 10:04
@davitol davitol mentioned this pull request Sep 3, 2019
Closed
13 tasks
@individual-it individual-it mentioned this pull request Sep 4, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mmattel mmattel mmattel left review comments

@settermjd settermjd Awaiting requested review from settermjd

Assignees

@DeepDiver1975 DeepDiver1975

Labels

3 - To Review

Projects

None yet

Milestone

development

Development

Successfully merging this pull request may close these issues.

4 participants

@DeepDiver1975 @settermjd @mmattel @micbar