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

Support extensions with multiple parts for gitlab (i.e. en.md) #1478

Merged
merged 1 commit into from
Jul 21, 2018
Merged

Support extensions with multiple parts for gitlab (i.e. en.md) #1478

merged 1 commit into from
Jul 21, 2018

Conversation

Nic128
Copy link

@Nic128 Nic128 commented Jun 30, 2018

- Summary
Fixes #1462. See discussion there for context.

- Test plan
I ported the exact fix from #1123 to gitlab.

- Description for the changelog
Support extensions with multiple parts for gitlab (i.e. en.md)

- A picture of a cute animal (not mandatory but encouraged)

@verythorough
Copy link
Contributor

Deploy preview for netlify-cms-www ready!

Built with commit c90b647

https://deploy-preview-1478--netlify-cms-www.netlify.com

@verythorough
Copy link
Contributor

verythorough commented Jun 30, 2018

Deploy preview for netlify-cms-www ready!

Built with commit bba97b2

https://deploy-preview-1478--netlify-cms-www.netlify.com

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

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

Awesome, just looks like a typo in the code.

this.fetchFiles(files.filter(file => fileExtension(file.name) === extension))
this.fetchFiles(files.filter(files =>
files.filter(file => file.name.endsWith('.' + extension))
)
Copy link
Contributor

@tech4him1 tech4him1 Jun 30, 2018

Choose a reason for hiding this comment

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

This whole block should be changed to:

this.fetchFiles(
  files.filter(file => file.name.endsWith('.' + extension))
)

(there was a duplicate files.filter(files => on the first line of the new code)

@verythorough
Copy link
Contributor

verythorough commented Jun 30, 2018

Deploy preview for cms-demo ready!

Built with commit bba97b2

https://deploy-preview-1478--cms-demo.netlify.com

@tech4him1 tech4him1 self-requested a review June 30, 2018 23:59
@Nic128
Copy link
Author

Nic128 commented Jul 2, 2018

Changes done.

@tech4him1
Copy link
Contributor

@Nic128 I'll try to get this merged later today or tomorrow

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

Thanks for this! The fileExtension helper import can be removed now, but other than that, 👍.

@Nic128
Copy link
Author

Nic128 commented Jul 7, 2018

There are more places that use the fileExtension helper. I'd rather not remove it in this PR.

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

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

Looks like the change needs made on line 82 as well (a new function since GitLab has pagination).

@Nic128
Copy link
Author

Nic128 commented Jul 7, 2018

Done.

@Nic128
Copy link
Author

Nic128 commented Jul 20, 2018

Is there more to do for this PR?

@tech4him1
Copy link
Contributor

tech4him1 commented Jul 20, 2018

So sorry, I've been pretty much offline for the last 2 weeks. I'll try to review ASAP.

@erquhart Maybe we can get this in the next patch release if possible?

@tech4him1 tech4him1 merged commit a801c06 into decaporg:master Jul 21, 2018
@tech4him1
Copy link
Contributor

@Nic128 Released in v1.9.4.

This pull request was closed.
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.

[Bug] Cannot list multi-part extension files
4 participants