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.

Sign up for GitHub

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

Jump to bottom

Add prefix option #86

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Add prefix option #86

wants to merge 13 commits into from

Conversation

lexuzieel
Copy link

This PR includes fixes and changes from feat-titlecase branch and closes #80

@lexuzieel lexuzieel mentioned this pull request Nov 14, 2022
Open
MichaelCurrin
MichaelCurrin requested changes Nov 21, 2022
View reviewed changes
Copy link
Owner

@MichaelCurrin MichaelCurrin left a comment

Choose a reason for hiding this comment

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

Some refactor and cleanup changes please

package.json Outdated Show resolved Hide resolved
src/test/suite/index.ts Outdated Show resolved Hide resolved
src/test/suite/index.ts Outdated Show resolved Hide resolved
src/test/suite/index.ts Outdated Show resolved Hide resolved
src/test/suite/index.ts Outdated Show resolved Hide resolved
MichaelCurrin
MichaelCurrin reviewed Nov 28, 2022
View reviewed changes
src/test/runTest.ts
}
}

main();
Copy link
Owner

Choose a reason for hiding this comment

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

Did you make the main function just to use async?

In newer Node you can use async/await without a function.

Copy link
Author

Choose a reason for hiding this comment

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

In my editor and when trying to run the test I receive the following error (running with node 16.14.2):

src/test/runTest.ts:25:3 - error TS1378: Top-level 'await' expressions are only allowed when the 'module' option is set to 'es2022', 'esnext', 'system', 'node16', or 'nodenext', and the 'target' option is set to 'es2017' or higher.

MichaelCurrin
MichaelCurrin reviewed Nov 28, 2022
View reviewed changes
src/test/runTest.ts Outdated Show resolved Hide resolved
MichaelCurrin
MichaelCurrin reviewed Dec 28, 2022
View reviewed changes
package.json Outdated Show resolved Hide resolved
@MichaelCurrin
Copy link
Owner

Hi, I am still going to look at this. I need to revert some of the commits in this before I can bring it in and I have been sorting out bug issues lately on the repo instead.

@lexuzieel
Copy link
Author

Hello. Sorry for my inactivity on this PR. I have read your comments, however I currently have no time to implement them.
Also you were right. After using this extension with my changes for some time it appears it does not capitalize properly in some cases (sometimes it broke after the first conversion). So it looks like it will need more work.

@lexuzieel
Copy link
Author

Hello 👋
I've merged upstream master branch with feat-prefix of my fork and corrected all but one issues with the code style (see the comment).
I guess now we somehow merge your PR with this one since it is related to a single feature and cover it with more test cases?

@lexuzieel lexuzieel requested a review from MichaelCurrin March 1, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelCurrin MichaelCurrin Awaiting requested review from MichaelCurrin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add an option to make messages capitalized
2 participants
@lexuzieel @MichaelCurrin