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

feat(words): add 'localtime' #786

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

sasakisasaki
Copy link
Contributor

As the title indicates 👍

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@HansRobo
Copy link
Member

Please provide a link to prove the word exists, as other developers have done?
This is important to clarify the origin of the word later.

@HansRobo
Copy link
Member

"Logiee" is already in the dictionary, so why do we need to add an "logiee-ss"?
I expect the hyphen split the word and not cause an error.

@sasakisasaki
Copy link
Contributor Author

@HansRobo Thank you so much for your review! Let me answer 👍

Please provide a link to prove the word exists, as other developers have done? This is important to clarify the origin of the word later.

Here the localtime is used.

"Logiee" is already in the dictionary, so why do we need to add an "logiee-ss"? I expect the hyphen split the word and not cause an error.

Oh, this is my misunderstanding! Yes, you are right, logiee-ss is not needed. Let me fix soon!

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki
Copy link
Contributor Author

@HansRobo Fixed by this commit 👍 . I do not see a logiee-ss related error in this workflow.

@HansRobo
Copy link
Member

HansRobo commented Sep 10, 2024

@sasakisasaki
Thank you for the fix!

Here the localtime is used.

Sorry, my explanation was confusing.
When I say "to prove the word exists," I don't mean to prove that the word exists in the place where you use it, but to prove that it exists in the public or industry and is worthy of being added to this repository, a "dictionary".

Normally, "localtime" is a compound word consisting of "local" and "time" and should be used with clear word separation such as "local time" or "LocalTime" or "local_time" and should not be added to the dictionary.

If you still must add it, you must give a reason why, and the fact that you are using it or that it is causing you errors is not a reason.

All you need to do here is provide a reliable source of information like https://linux.die.net/man/3/localtime that explains the fact that in the context of Linux and C, the compound word "localtime" is commonly used without any separators.

This has become a long message, but it would be very helpful if you could pay attention to these things next time.
As for "localtime", I understand that it is commonly used in the industry, so there is no problem in adding it.

You can read my point here too: https://github.com/tier4/autoware-spell-check-dict/blob/main/CONTRIBUTING.md

@HansRobo HansRobo changed the title feat(words): add 'localtime' and 'logiee-ss' feat(words): add 'localtime' Sep 10, 2024
@HansRobo
Copy link
Member

HansRobo commented Sep 10, 2024

FYI, when you want to add a word, you can use a useful workflow.
https://github.com/tier4/autoware-spell-check-dict#easy-to-add-a-word-using-workflow
(A PR created with this workflow will have a TODO asking you to provide a source link.)

@sasakisasaki
Copy link
Contributor Author

@HansRobo Thanks, that really makes sense.

If you still must add it, you must give a reason why, and the fact that you are using it or that it is causing you errors is not a reason.
All you need to do here is provide a reliable source of information like https://linux.die.net/man/3/localtime that explains the fact that in the context of Linux and C, the compound word "localtime" is commonly used without any separators.

I got it and I have read the guideline for contribution 👍

FYI, when you want to add a word, you can use a useful workflow.
https://github.com/tier4/autoware-spell-check-dict#easy-to-add-a-word-using-workflow

Oh! This is so useful! Thank you very much @HansRobo !

@sasakisasaki sasakisasaki merged commit a773009 into main Sep 10, 2024
6 checks passed
@sasakisasaki sasakisasaki deleted the add-words-logiee-ss-on-docker-related-ones branch September 10, 2024 10:08
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