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

Create new array-based RSS feed config structure #1258

Merged
merged 12 commits into from
Mar 12, 2023

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Mar 12, 2023

Updates the RSS configuration array settings

Development

I believe this to be the (or one of the) last breaking change as HydePHP enters GA in two days.
While this is a breaking change, the impact is very low. It's a really easy change, and I don't think that many people have changed this setting, and the BC has not fully kicked in yet, so I think the risk is acceptable.

Changes the following in config/hyde.php:

// Should the RSS feed be generated?
'generate_rss_feed' => true,

// What filename should the RSS file use?
'rss_filename' => 'feed.xml',

// The channel description.
'rss_description' =>  env('SITE_NAME', 'HydePHP').' RSS Feed',

To instead be like this:

'rss' => [
    // Should the RSS feed be generated?
    'enabled' => true,

    // What filename should the RSS file use?
    'filename' => 'feed.xml',

    // The channel description.
    'description' => env('SITE_NAME', 'HydePHP').' RSS Feed',
],

Benefits & Drawbacks

The benefits of changing the config file structure to use nested arrays as shown in the second code block are:

  1. Clarity and organization: Using nested arrays to group related configuration options makes it easier to understand the purpose and organization of the options, which can improve the overall clarity of the codebase.

  2. Flexibility: By grouping related options into a nested array, it can make it easier to add additional related options in the future.

  3. Reduced potential for naming conflicts: Grouping related options into a nested array can help prevent naming conflicts between options that may have similar or identical names.

The drawbacks of changing the config file structure to use nested arrays are:

  1. Increased complexity: The use of nested arrays can add complexity to the codebase, as developers may need to navigate multiple levels of arrays to access the desired configuration option.

  2. Backwards compatibility issues: If this is a widely used configuration file, changing its structure can create compatibility issues with existing code that expects the original structure.

  3. Increased verbosity: The use of nested arrays can make the code more verbose, especially when accessing the configuration values. In effect, however, this is not a concern as rss_description is not much different than rss.description.

  4. Visual discrepancy with the generate_sitemap option

This section was written by ChatGPT (excluding point 4)

@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:17 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Mar 12, 2023

Codecov Report

❗ No coverage uploaded for pull request base (master@18f7c25). Click here to learn what that means.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             master     #1258   +/-   ##
==========================================
  Coverage          ?   100.00%           
  Complexity        ?      2632           
==========================================
  Files             ?       304           
  Lines             ?      6908           
  Branches          ?         0           
==========================================
  Hits              ?      6908           
  Misses            ?         0           
  Partials          ?         0           
Impacted Files Coverage Δ
packages/framework/src/Facades/Features.php 100.00% <100.00%> (ø)
...mework/Features/XmlGenerators/RssFeedGenerator.php 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:20 — with GitHub Actions Inactive
@caendesilva caendesilva force-pushed the finalize-rss-configuration branch from 0674d68 to e85dac0 Compare March 12, 2023 20:22
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:22 — with GitHub Actions Inactive
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:25 — with GitHub Actions Inactive
@caendesilva caendesilva force-pushed the finalize-rss-configuration branch from 7d0c7ce to 93fd546 Compare March 12, 2023 20:27
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:27 — with GitHub Actions Inactive
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:28 — with GitHub Actions Inactive
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:28 — with GitHub Actions Inactive
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:30 — with GitHub Actions Inactive
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:31 — with GitHub Actions Inactive
@caendesilva caendesilva temporarily deployed to pr-documentation-1258 March 12, 2023 20:32 — with GitHub Actions Inactive
@caendesilva caendesilva marked this pull request as ready for review March 12, 2023 20:33
@caendesilva caendesilva merged commit 8c59238 into master Mar 12, 2023
@caendesilva caendesilva deleted the finalize-rss-configuration branch March 12, 2023 20:38
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.

1 participant