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

Configure default naming convention for target files #161

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Droxx
Copy link

@Droxx Droxx commented May 8, 2023

Added the ability to configure a target naming convention. When this is set, converted files will have a naming convention applied to the target files.

Supported conventions include
PascalCase
camelCase
kebab-case
snake_case

I implemented this because I work with a team that are generating configuration files for our software. Some team members prefer to work in YAML, and some prefer to work in JSON. The software that consumes these configuration files is agnostic towards the file format. However, when files are in YAML format, it is not case insensitive.
When team members convert a file, (we have been using YAML<3JSON for some time!). The act of adjusting the naming convention from a case-insensitive JSON file, to a camelCase YAML file can be tedious.
This change can overcome that hurdle, and have all, newly-converted files, adhere to the a camelCase naming convention instantly upon conversion.

This could be useful for similar use cases elsewhere. I know for certain that the most widely used YAML deserializer package in .NET is case sensitive, whereas the JSON serializers all have an option to ignore case. So any developers out there with equally agnostic software could benefit from this change.

@Droxx Droxx requested a review from hilleer as a code owner May 8, 2023 09:05
@socket-security
Copy link

socket-security bot commented May 8, 2023

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

@hilleer
Copy link
Owner

hilleer commented May 18, 2023

Hi @Droxx

Thanks for opening this pull request - you're officially the first contributor for the extension beside @dependabot and me 🥳

Can you provide some examples of the use-cases of this in the pull-request description, please? 🙂

@Droxx
Copy link
Author

Droxx commented May 19, 2023

Hi @Droxx

Thanks for opening this pull request - you're officially the first contributor for the extension beside @dependabot and me 🥳

Can you provide some examples of the use-cases of this in the pull-request description, please? 🙂

Sure! Updated, thankyou

@hilleer
Copy link
Owner

hilleer commented May 22, 2023

Hi @Droxx
Thanks for opening this pull request - you're officially the first contributor for the extension beside @dependabot and me 🥳
Can you provide some examples of the use-cases of this in the pull-request description, please? 🙂

Sure! Updated, thankyou

Great, I like the updated description! Thanks. Can we add just a few examples of what a converted file name becomes, like this: FOO_BAR.json --> fooBar.yaml with a few of the options? 👀

Also, are you able to resolve the merge conflicts of the pull-request?

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/helpers.ts Outdated Show resolved Hide resolved
src/helpers.ts Outdated Show resolved Hide resolved
@Droxx
Copy link
Author

Droxx commented May 22, 2023

Thanks for the feedback @hilleer. I've addressed most of it but I got interrupted! I'll finish it off and push a resolving commit (and resolved conflicts). Then I'll nudge you

@Droxx
Copy link
Author

Droxx commented May 22, 2023

All done and ready for re-review @hilleer

@hilleer
Copy link
Owner

hilleer commented May 22, 2023

All done and ready for re-review @hilleer

Great! Good job. I'll be going on an offsite with work. Will get back to a re-review asap!

@Droxx
Copy link
Author

Droxx commented May 30, 2023

Hi @hilleer, just adding a polite reminder here for a re-review

@hilleer
Copy link
Owner

hilleer commented May 31, 2023

Hi @hilleer, just adding a polite reminder here for a re-review

Hi @Droxx

Thanks for the reminder! Re-review is on my list but I'm quite busy at the moment. Will get back to this ASAP, don't worry!

@socket-security
Copy link

New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives1 Size Publisher
change-case 🆕 4.1.2 None +15 252 kB blakeembrey

Footnotes

  1. https://docs.socket.dev

src/conventions.ts Outdated Show resolved Hide resolved
@hilleer
Copy link
Owner

hilleer commented Jun 23, 2023

Hi @Droxx

Once again big thanks for your contribution! 🥳

While reviewing the code I couldn't stop thinking wether this feature actually belongs in this extension and the purpose of it 🤔 It somewhat feel like an edge case and something that might be part of a different- or its own extension.

Leaving this here for a bit to see if other users of the extension or from the community will share their thoughts on this.

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