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

Improve the Yaml configuration loader #1770

Merged
merged 35 commits into from
Jul 1, 2024

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Jun 30, 2024

Copy link

codecov bot commented Jun 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (ab9519f) to head (b14d20b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##              master     #1770   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity      1750      1758    +8     
===========================================
  Files            180       180           
  Lines           4674      4689   +15     
===========================================
+ Hits            4674      4689   +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@caendesilva caendesilva force-pushed the improve-yaml-configuration-loading branch from 06aff63 to e4e744a Compare June 30, 2024 18:52
@caendesilva caendesilva force-pushed the improve-yaml-configuration-loading branch from e4e744a to 0b7b339 Compare June 30, 2024 18:52
Squashed commit of the following:

commit 2b9d8d1
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 15:35:17 2024 +0200

    Refactor internal config handling to use a property

commit 5af1fea
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 13:18:21 2024 +0200

    Introduce local variable

commit f12314a
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 13:11:21 2024 +0200

    Configure site name environment variable from Yaml config

commit 7942491
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 13:04:06 2024 +0200

    Draft new method to merge environment variables

commit d32d688
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 13:00:14 2024 +0200

    Introduce property for the loaded data

commit 301a9bc
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 12:56:49 2024 +0200

    Skip the failing tests

commit 65b2a05
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 12:55:55 2024 +0200

    Add todo

commit ebe9146
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 12:52:00 2024 +0200

    Test setting site name merges sidebar header

commit 3e02dc5
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 12:45:50 2024 +0200

    Create failing test

commit 2f18f95
Author: Caen De Silva <caen@desilva.se>
Date:   Fri Apr 26 12:44:33 2024 +0200

    Draft initial tests
@caendesilva caendesilva force-pushed the improve-yaml-configuration-loading branch from b91232c to 4eb35db Compare June 30, 2024 19:03
@caendesilva caendesilva force-pushed the improve-yaml-configuration-loading branch from 566b146 to d8bcfcd Compare June 30, 2024 19:34
@caendesilva caendesilva force-pushed the improve-yaml-configuration-loading branch from 96e92a5 to 6856110 Compare June 30, 2024 19:39
@caendesilva
Copy link
Member Author

Unless this is modified to another pattern, we should also support this

    'description' => env('SITE_NAME', 'HydePHP').' RSS Feed',

And of course this

    Meta::property('site_name', env('SITE_NAME', 'HydePHP')),

I think these are all the remaining ones

@caendesilva
Copy link
Member Author

It's a bit hard to test, but I confirmed manually that setting the site name in the environment variables, that takes precedence over all this, as those are evaluated before this.

@caendesilva caendesilva force-pushed the improve-yaml-configuration-loading branch from 31bf372 to 32e3772 Compare July 1, 2024 13:27
@caendesilva
Copy link
Member Author

caendesilva commented Jul 1, 2024

    Meta::property('site_name', env('SITE_NAME', 'HydePHP')),

Not sure how we can set this, since it's not keyed.

Edit: We can probably loop through the array, but this is starting to get complex. I think adding a dedicated environment configuration loader to get data from the Yaml file could work if it's in between the environment and configuration loaders

@caendesilva caendesilva marked this pull request as ready for review July 1, 2024 13:49
@caendesilva caendesilva merged commit f8f60cd into master Jul 1, 2024
24 checks passed
@caendesilva caendesilva deleted the improve-yaml-configuration-loading branch July 1, 2024 13:52
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.

Configuration loader should be able to set documentation sidebar title from Hyde YAML config
1 participant