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 an empty line! #634

Closed
wants to merge 3 commits into from
Closed

Remove an empty line! #634

wants to merge 3 commits into from

Conversation

guledali
Copy link
Contributor

No description provided.

@guledali
Copy link
Contributor Author

@peterp how make the CI pass?

@thedavidprice
Copy link
Contributor

Hi @guledali! Thanks for this. Is the reason for making this change because the generated file is throwing an ESLint error right away?

For reference, here's the CI test error:

@redwoodjs/cli:     -
@redwoodjs/cli:       import Posts from 'src/components/Posts'
@redwoodjs/cli:       export const QUERY = gql`
@redwoodjs/cli:         query POSTS {
@redwoodjs/cli:           posts {
@redwoodjs/cli: 
@redwoodjs/cli:       109 |   expect(
@redwoodjs/cli:       110 |     files['/path/to/project/web/src/components/PostsCell/PostsCell.js']
@redwoodjs/cli:     > 111 |   ).toEqual(
@redwoodjs/cli:           |     ^
@redwoodjs/cli:       112 |     loadGeneratorFixture('scaffold', path.join('components', 'indexCell.js'))
@redwoodjs/cli:       113 |   )
@redwoodjs/cli:       114 | })
@redwoodjs/cli: 
@redwoodjs/cli:       at Object.<anonymous> (src/commands/generate/scaffold/__tests__/scaffold.test.js:111:5)

@cannikin looks like we might need your help here -- I've no idea why removing the empty line would break a path in the test?

@cannikin
Copy link
Member

cannikin commented Jun 1, 2020

I've no idea why removing the empty line would break a path in the test?

That's looking at the content of the file that's generated and comparing it against the fixture file. So if the generated file now has one less line break it won't match the fixture.

We used to have an eslint rule that put empty lines between imports from packages and imports from other paths in the app itself...did those rules get changed? I had that empty line in there so there wouldn't be any changes between what came out of the generator and what the linter would fix if you went into the file and saved it yourself...

@guledali
Copy link
Contributor Author

guledali commented Jun 1, 2020

We used to have an eslint rule that put empty lines between imports from packages and imports from other paths in the app itself...did those rules get changed?

I think so
Screenshot 2020-06-01 at 19 05 25

@cannikin
Copy link
Member

cannikin commented Jun 1, 2020

Ahh interesting. Is this the only generated file that has that empty line?

@guledali
Copy link
Contributor Author

guledali commented Jun 1, 2020

@cannikin Just checked that's the only file

@cannikin
Copy link
Member

cannikin commented Jun 1, 2020

Awesome! You'll just have to update the text fixture for that file and remove the newline from that as well. Locally you can cd into the cli package and yarn test to check that it's working.

@peterp peterp mentioned this pull request Jun 2, 2020
5 tasks
@peterp peterp changed the base branch from master to main June 19, 2020 15:33
@guledali
Copy link
Contributor Author

@jtoar, I'm closing this one, not much of change anyway and + new templates are coming soon #686 (comment)

@guledali guledali closed this Jun 19, 2020
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.

3 participants