Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Layout xml #2051

Closed
wants to merge 3 commits into from
Closed

Layout xml #2051

wants to merge 3 commits into from

Conversation

buskamuza
Copy link
Contributor

This PR is a:

  • New topic
  • Content fix or rewrite
  • Bug fix or improvement

Summary

Original PR - #1078

The goal of this is to formalize a standard for writing layout XML. There hasn't been a standard set that the community follows when authoring layout XML. By setting a precedent for layout XML naming conventions, I am hopeful that it will lead to XML declarations that are easier to read and more consistent which could decrease maintenance time.
Note: this is identical to #982. I found that I did that PR on the wrong branch of my fork and needed to move over to this one to provide flexibility.

Additional information

JesseMaxwell and others added 3 commits March 25, 2017 13:03
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jun 1, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@buskamuza buskamuza mentioned this pull request Jun 1, 2018
Closed
@bassplayer7
Copy link
Contributor

bassplayer7 commented Jun 6, 2018 •
edited
Loading

Thanks for the work on this. I attempted to sign the CLA but it doesn't seem to make a difference. (It decided to update)

bassplayer7
bassplayer7 reviewed Jun 6, 2018
View reviewed changes
guides/v2.1/coding-standards/layout-coding-standard.md
The `name` attribute should adhere to the guidelines listed below. The `name` attribute should:

- Should explain element's purpose rather than its position on the page .
- Be unique to the module that declares it by prepending it with module's namespace as `<vendor>.<module>`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I like this idea. However, I'm a bit hesitant because literally none of the core does this. I was hoping to keep it compatible with at least the general principle of what is already in the core.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be done for the new layouts. So far it's just my idea, I'll be discussing it more. Thanks for your feedback.

@buskamuza
Copy link
Contributor Author

I guess it wants me to sign CLA :)

magento-cicd2 pushed a commit that referenced this pull request Jun 28, 2018
@jfrontain
Merge pull request #2051 from magento/2.2.5-attribution-correction
53128f5 
Added community member attribution to 2.2.5 release notes
@jcalcaben jcalcaben changed the base branch from develop to master July 6, 2018 17:39
@jeff-matthews
Copy link
Contributor

@jcalcaben, can we merge this?

@jcalcaben
Copy link
Contributor

@buskamuza is there a status update for this PR?

@bassplayer7
Copy link
Contributor

bassplayer7 commented Jul 25, 2018 •
edited
Loading

Suggestion: what about removing this line for now? We can always add it later if the core starts drifting in this direction.

Be unique to the module that declares it by prepending it with module's namespace as <vendor>.<module>

@buskamuza
Copy link
Contributor Author

@melnikovi , @antonkril , please review

@buskamuza
Copy link
Contributor Author

Closing the proposal based on the discussion https://github.com/magento/architecture/wiki/October-31,-2018

@buskamuza buskamuza closed this Oct 31, 2018
@lorikrell lorikrell added New Topic A major update published as an entirely new document and removed New doc labels Mar 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bassplayer7 bassplayer7 bassplayer7 left review comments

Assignees

@buskamuza buskamuza

@jcalcaben jcalcaben

Labels
2.1.x New Topic A major update published as an entirely new document
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@buskamuza @magento-cicd2 @bassplayer7 @jeff-matthews @jcalcaben @dshevtsov @lorikrell @JesseMaxwell