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

Remove frontend_type tag for text #2148

Merged
merged 1 commit into from
Jun 8, 2022
Merged

Remove frontend_type tag for text #2148

merged 1 commit into from
Jun 8, 2022

Conversation

luigifab
Copy link
Contributor

@luigifab luigifab commented May 25, 2022

Description

For each system.xml file in module directory, we have:

<config>
  <sections>
    <example1>
      <label>Example</label>
      <frontend_type>text</frontend_type><!-- useless -->
      <groups>
        <example2>
          <label>Example</label>
          <frontend_type>text</frontend_type><!-- useless -->
          <fields>
            <example3>
              <label>Example</label>
              <frontend_type>text</frontend_type><!-- this is the default value -->
              <sort_order>1</sort_order>
              <show_in_default>1</show_in_default>
              <show_in_website>1</show_in_website>
              <show_in_store>1</show_in_store>

For example1 and example2 tags, <frontend_type>text</frontend_type> are not needed.
For example3 tag, <frontend_type>text</frontend_type> is the default value, so not really required.

So, because it's more easy to remove all, I removed all tags.
Moreover, this will reduce the size of the merged XML in memory.
After #1916, this will also reduce the size of the cache.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Component: AdminNotification Relates to Mage_AdminNotification Component: Api PageRelates to Mage_Api Component: Authorizenet Relates to Mage_Authorizenet Component: Backup Relates to Mage_Backup Component: Captcha Relates to Mage_Captcha Component: Catalog Relates to Mage_Catalog Component: CatalogInventory Relates to Mage_CatalogInventory Component: CatalogSearch Relates to Mage_CatalogSearch Component: Centinel Relates to Mage_Centinel Component: Checkout Relates to Mage_Checkout Component: Cms Relates to Mage_Cms Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Contacts Relates to Mage_Contacts Component: Core Relates to Mage_Core Component: Cron Relates to Mage_Cron Component: Customer Relates to Mage_Customer Component: Directory Relates to Mage_Directory Component: Downloadable Relates to Mage_Downloadable Component: GoogleAnalytics Relates to Mage_GoogleAnalytics Component: ImportExport Relates to Mage_ImportExport Component: Log Relates to Mage_Log Component: Oauth Relates to Mage_Oauth Component: Page Relates to Mage_Page Component: PageCache Relates to Mage_PageCache Component: Paygate Relates to Mage_Paygate Component: Payment Relates to Mage_Payment Component: PayPal Relates to Mage_Paypal Component: ProductAlert Relates to Mage_ProductAlert Component: Reports Relates to Mage_Reports Component: Review Relates to Mage_Review labels May 25, 2022
@github-actions github-actions bot added Component: Rss Relates to Mage_Rss Component: Sales Relates to Mage_Sales Component: SalesRule Relates to Mage_SalesRule Component: Sendfriend Relates to Mage_Sendfriend Component: Shipping Relates to Mage_Shipping Component: Sitemap Relates to Mage_Sitemap Component: Tax Relates to Mage_Tax Component: Usa Relates to Mage_Usa Component: Weee Relates to Mage_Weee Component: Wishlist Relates to Mage_Wishlist labels May 25, 2022
fballiano
fballiano previously approved these changes Jun 7, 2022
@luigifab
Copy link
Contributor Author

luigifab commented Jun 7, 2022

I will rebase on top of 1.9.4.x for added configurations.

@addison74
Copy link
Contributor

May I suggest a small change in PR description? "For each system.xml file in a Magento module directory ...".

I did not test the PR but it is interesting the observation made by you related to the uselessness of those tags in the cascade. I'm sure you did a simulation after which you realized this issue.

@luigifab
Copy link
Contributor Author

luigifab commented Jun 8, 2022

I realized that Magento use this tag everywhere in #2086.
So I removed it everywhere :)

Copy link
Member

@elidrissidev elidrissidev left a comment

Choose a reason for hiding this comment

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

Seems good to me. Definitely a worth it improvement!

@fballiano fballiano merged commit c83207b into OpenMage:1.9.4.x Jun 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2022

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit c83207b. ± Comparison against base commit 5761ade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: AdminNotification Relates to Mage_AdminNotification Component: Api PageRelates to Mage_Api Component: Authorizenet Relates to Mage_Authorizenet Component: Backup Relates to Mage_Backup Component: Captcha Relates to Mage_Captcha Component: Catalog Relates to Mage_Catalog Component: CatalogInventory Relates to Mage_CatalogInventory Component: CatalogSearch Relates to Mage_CatalogSearch Component: Centinel Relates to Mage_Centinel Component: Checkout Relates to Mage_Checkout Component: Cms Relates to Mage_Cms Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Contacts Relates to Mage_Contacts Component: Core Relates to Mage_Core Component: Cron Relates to Mage_Cron Component: Customer Relates to Mage_Customer Component: Directory Relates to Mage_Directory Component: Downloadable Relates to Mage_Downloadable Component: GoogleAnalytics Relates to Mage_GoogleAnalytics Component: ImportExport Relates to Mage_ImportExport Component: Log Relates to Mage_Log Component: Oauth Relates to Mage_Oauth Component: Page Relates to Mage_Page Component: PageCache Relates to Mage_PageCache Component: Paygate Relates to Mage_Paygate Component: Payment Relates to Mage_Payment Component: PayPal Relates to Mage_Paypal Component: ProductAlert Relates to Mage_ProductAlert Component: Reports Relates to Mage_Reports Component: Review Relates to Mage_Review Component: Rss Relates to Mage_Rss Component: Sales Relates to Mage_Sales Component: SalesRule Relates to Mage_SalesRule Component: Sendfriend Relates to Mage_Sendfriend Component: Shipping Relates to Mage_Shipping Component: Sitemap Relates to Mage_Sitemap Component: Tax Relates to Mage_Tax Component: Usa Relates to Mage_Usa Component: Weee Relates to Mage_Weee Component: Wishlist Relates to Mage_Wishlist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants