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.

Sign up for GitHub

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

Jump to bottom

Add sample OIDC configuration to the sample app config file #36526

Merged
merged 1 commit into from
Feb 4, 2020
Merged

Add sample OIDC configuration to the sample app config file #36526

merged 1 commit into from
Feb 4, 2020

Conversation

settermjd
Copy link
Contributor

Description

Add sample OIDC (OpenID Connect) configuration to the sample app config file

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

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)
  • Documentation

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@settermjd settermjd added this to the development milestone Dec 5, 2019
@settermjd settermjd requested a review from phil-davis December 5, 2019 11:46
@settermjd settermjd self-assigned this Dec 5, 2019
@mmattel mmattel mentioned this pull request Dec 5, 2019
Closed
12 tasks
DeepDiver1975
DeepDiver1975 requested changes Dec 5, 2019
View reviewed changes
config/config.apps.sample.php Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 5, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #36526   +/-   ##
=========================================
  Coverage      64.7%    64.7%           
  Complexity    19130    19130           
=========================================
  Files          1270     1270           
  Lines         74868    74868           
  Branches       1329     1329           
=========================================
  Hits          48447    48447           
  Misses        26030    26030           
  Partials        391      391
Flag Coverage Δ Complexity Δ
#javascript 54.17% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.28% <ø> (-0.43%) 19130 <ø> (ø)
Impacted Files Coverage Δ Complexity Δ
lib/private/DB/AdapterOCI8.php 86.66% <0%> (ø) 4% <0%> (ø) ⬇️
...ivate/Files/ObjectStore/HomeObjectStoreStorage.php 86.66% <0%> (ø) 8% <0%> (ø) ⬇️
...Builder/ExpressionBuilder/OCIExpressionBuilder.php 85.18% <0%> (ø) 18% <0%> (ø) ⬇️
lib/private/DB/OracleMigrator.php 76.84% <0%> (ø) 10% <0%> (ø) ⬇️
lib/private/DB/OracleConnection.php 66.66% <0%> (ø) 12% <0%> (ø) ⬇️
lib/private/Files/Storage/Wrapper/Availability.php 50.64% <0%> (ø) 80% <0%> (ø) ⬇️
lib/private/Files/ObjectStore/NoopScanner.php 87.5% <0%> (ø) 6% <0%> (ø) ⬇️
lib/private/Preview/MP3.php 58.06% <0%> (ø) 13% <0%> (ø) ⬇️
lib/private/Autoloader.php 92.68% <0%> (ø) 20% <0%> (ø) ⬇️
...b/private/Files/ObjectStore/ObjectStoreStorage.php 77.24% <0%> (ø) 105% <0%> (ø) ⬇️
... and 23 more

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 4cf252a...9c0460c. Read the comment docs.

phil-davis
phil-davis reviewed Dec 9, 2019
View reviewed changes
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Language looks OK. I am not qualified to know if the detailed content is correct/complete.

@mmattel
Copy link
Contributor

mmattel commented Dec 9, 2019

@DeepDiver1975 👋 mind to re-review so we could merge ?
Because of other changes in config.sample.php, a config-to docs run needs to be done and I want to do it in one shot...

DeepDiver1975
DeepDiver1975 requested changes Dec 11, 2019
View reviewed changes
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
config/config.apps.sample.php Outdated Show resolved Hide resolved
@phil-davis
Copy link
Contributor

@settermjd I don't see where changes have been pushed yet

DeepDiver1975
DeepDiver1975 requested changes Dec 12, 2019
View reviewed changes
Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

please respect given comments - THX

@mmattel
Copy link
Contributor

mmattel commented Jan 18, 2020

@settermjd
Because these config values belong to a particular app which are not part of core or part of apps bundled with core in the tar file, you need to write which app is needed for these config values.

/**
 * OIDC Configuration  (Which App??)
 */

Please see the examples in the file. If there is no app needed, you have to move this to the standard config.sample file into the correct section.

@mmattel mmattel mentioned this pull request Jan 22, 2020
Merged
@mmattel
Copy link
Contributor

mmattel commented Jan 22, 2020

@settermjd ping, any update?

DeepDiver1975
DeepDiver1975 previously requested changes Jan 27, 2020
View reviewed changes
Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

Looks good now - THX

  • squash
  • make drone happy (code style is wrong)

@settermjd
Copy link
Contributor Author

Looks good now - THX

* [ ]  squash

* [ ]  make drone happy (code style is wrong)

shall do.

@settermjd settermjd closed this Jan 27, 2020
@settermjd settermjd reopened this Jan 27, 2020
@settermjd settermjd mentioned this pull request Jan 27, 2020
Closed
12 tasks
@micbar
Copy link
Contributor

micbar commented Jan 27, 2020

@settermjd please use the shorthand array format [] instead of array()

@phil-davis
Copy link
Contributor

@settermjd ping
This likely only needs you to make minor fixes and squash.

@settermjd @phil-davis
Add sample OIDC configuration to the sample app config file
9c0460c 
This relates to owncloud/docs#2110.
It uses a complete sample to avoid creating two samples or a messy
docblock element.
@phil-davis phil-davis dismissed DeepDiver1975’s stale review February 4, 2020 09:37

squashed and fixed array [] format

@phil-davis phil-davis merged commit 38ec7de into master Feb 4, 2020
@delete-merged-branch delete-merged-branch bot deleted the 2100-document-oidc branch February 4, 2020 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmattel mmattel mmattel approved these changes

@phil-davis phil-davis phil-davis approved these changes

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

Assignees

@settermjd settermjd

Labels
4 - To release documentation
Projects
None yet
Milestone
development
Development

Successfully merging this pull request may close these issues.
5 participants
@settermjd @mmattel @phil-davis @micbar @DeepDiver1975