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 consistency of block names #1593

Merged
merged 10 commits into from
Jun 30, 2017
Merged

Improve consistency of block names #1593

merged 10 commits into from
Jun 30, 2017

Conversation

nylen
Copy link
Member

@nylen nylen commented Jun 29, 2017

There are a few blocks that have sub-optimal names due to our server-side parsing not handling dashes properly:

  • core/latestposts should be core/latest-posts
  • core/embedcollegehumor (and other embed blocks) should be core/embed-collegehumor or core-embed/collegehumor. Per Slack discussion with @mtias, and because there are enough of these blocks that it makes sense for them to live in their own "namespace", I've gone with the latter.

With the new PHP parser in #1152 we can resolve this ambiguity, with plenty of test cases, and set good standards for other block implementers.

@nylen
Copy link
Member Author

nylen commented Jun 29, 2017

#882 is the related issue I was looking for earlier.

It's probably easiest to review this PR without the fixture file changes and without #1152 in the mix, as follows:

git diff 2f8f07b...0074b3c blocks/library/ blocks/test/full-content.js

@notnownikki
Copy link
Member

As far as the renaming goes this is 👍 from me, this makes much more sense than the smooshed together names.

I'm not familiar yet with how pegjs works, so I wont mark this as approved, just because I don't yet know enough to properly review that part.

@nylen
Copy link
Member Author

nylen commented Jun 29, 2017

That's fine, that's a separate PR anyway (#1152).

@nylen nylen force-pushed the update/improve-block-names branch from cab018b to c140f30 Compare June 30, 2017 03:07
@nylen
Copy link
Member Author

nylen commented Jun 30, 2017

Merging to get this breaking change in sooner rather than later.

@nylen nylen closed this Jun 30, 2017
@nylen nylen reopened this Jun 30, 2017
@nylen nylen merged commit 9a5ee49 into master Jun 30, 2017
@nylen nylen deleted the update/improve-block-names branch June 30, 2017 03:22
@mtias mtias added [Feature] Block API API that allows to express the block paradigm. [Feature] Blocks Overall functionality of blocks [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f labels Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Blocks Overall functionality of blocks [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants