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

Ensure that all block fixtures end with newlines #2034

Merged
merged 1 commit into from
Jul 26, 2017

Conversation

nylen
Copy link
Member

@nylen nylen commented Jul 26, 2017

Follow-up to #1929. The whitespace in these fixture files has been flipping back and forth a bit, let's put a stop to that.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Verified that there are no modified files when running:

gutenberg (fix/block-fixtures-newlines) $ npx rimraf blocks/test/fixtures/*.serialized.html
gutenberg (fix/block-fixtures-newlines) $ npx rimraf blocks/test/fixtures/*.json
gutenberg (fix/block-fixtures-newlines) $ GENERATE_MISSING_FIXTURES=y npm run test-unit

const serializedActual = serialize( blocksActual );
// `serialize` doesn't have a trailing newline, but the fixture
// files should.
const serializedActual = serialize( blocksActual ) + '\n';
Copy link
Member

Choose a reason for hiding this comment

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

Should we trim here in case serialize changes in the future to add its own trailing newline or... update this logic if/when that time comes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Update when the time comes; require an exact match so that we know if this behavior changes.

I think the current behavior is correct for post_content, FWIW.

@nylen nylen merged commit 834ecf5 into master Jul 26, 2017
@nylen nylen deleted the fix/block-fixtures-newlines branch July 26, 2017 17:31
@nylen
Copy link
Member Author

nylen commented Jul 26, 2017

Eh, probably should have waited for the CI build to pass before merging, but I did the same verification locally.

@aduth
Copy link
Member

aduth commented Jul 27, 2017

In following up on #1929 (comment), I'm finding that .serialized.html files are generated with two ending newlines now 😅

@nylen
Copy link
Member Author

nylen commented Jul 27, 2017

         ︵ ┻━━┻
(╯°□°)╯     u\

@nylen
Copy link
Member Author

nylen commented Jul 27, 2017

I suppose this is because the initial HTML files contain a trailing newline, and so we should remove the
+ '\n' introduced here.

ceyhun pushed a commit that referenced this pull request Apr 22, 2020
Enable Cover Block for Production
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.

2 participants