Skip to content

fix(cli): Generate file names with proper casing#450

Merged
NickSeagull merged 12 commits intoboostercloud:masterfrom
verogarp:generate-file-names-with-proper-casing
Oct 14, 2020
Merged

fix(cli): Generate file names with proper casing#450
NickSeagull merged 12 commits intoboostercloud:masterfrom
verogarp:generate-file-names-with-proper-casing

Conversation

@verogarp
Copy link
Contributor

Description

Fixes #306

Changes

Used inflection for modify the names of files with underscore function, and then replace underscore by dashes.

Checks

  • Project Builds
  • Project passes tests and checks
  • Updated documentation accordingly

Additional information

Integration tests for CLI dont work, see #449

@verogarp verogarp force-pushed the generate-file-names-with-proper-casing branch from 161376a to 4f4bab0 Compare October 13, 2020 10:56
Copy link
Member

@javiertoledo javiertoledo left a comment

Choose a reason for hiding this comment

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

Hey @verogarp, thanks a lot for this, this change is 💯, super useful to keep consistency with the language standards. You also helped us to notice that we're using an inflection library that doesn't seem maintained anymore, so It could make a lot of sense to replace it with inflected if it works better (I haven't tried it myself). If you want to change it in this PR I'd be happy to review it again, but if you don't have the time, could you please create a new issue to come back to this later? Thanks a lot!

commaSeparatedComponents: eventData.eventName,
}))
const eventsImports: Array<ImportDeclaration> = info.events.map((eventData) => {
const fileName = inflection.underscore(eventData.eventName).replace(/_/g, '-')
Copy link
Member

Choose a reason for hiding this comment

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

Oh! When I saw this line I thought that inflection.dasherize should do the job better, but I tried and... surprise! It doesn't! It looks like other people had the same issue and there are even PRs to solve it, but the repository seems abandoned dreamerslab/node.inflection#72

You could avoid manually replacing underscores with dashes doing something like this:

Suggested change
const fileName = inflection.underscore(eventData.eventName).replace(/_/g, '-')
const fileName = inflection.transform( eventData.eventName, [ 'underscore', 'dasherize' ])

But maybe a better choice would be to try the library inflected (https://www.npmjs.com/package/inflected), which looks like a more up to date drop-in replacement. If you want to check if it works better, it could make sense to replace it in the whole project... (you'll likely need to do some other changes in other files, I bet the compiler will guide you, but if you need help, don't hesitate to ask in Discord 😉)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @javiertoledo ! I can't find the "transform" function so I use "dasherize" and "underscore". You can see it here:

const fileName = inflected.dasherize(inflected.underscore(eventData.eventName))

Copy link

@NickSeagull NickSeagull 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 the PR 😄

Can you please refactor all the equal code into a unique file, so we only have one place to change it? This will help maintaining the project

You can do this in another PR and just get this one merged

commaSeparatedComponents: projection.entityName,
}))
const eventsImports: Array<ImportDeclaration> = info.projections.map((projection) => {
const fileName = inflected.dasherize(inflected.underscore(projection.entityName))

Choose a reason for hiding this comment

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

We are using the same code in 4 different places, can you create a new file under cli/src/common called filenames.ts that with a function called classNameToFileName that does this, but only in one place?

@NickSeagull NickSeagull merged commit cc7f5fb into boostercloud:master Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make generators generate file names with proper casing

3 participants