-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Update gun database #31
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
Conversation
Summary of ChangesHello @tinoue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on maintaining the accuracy and completeness of the gun catalog database. It includes minor corrections to existing entries to ensure proper Japanese naming conventions and introduces a new product entry, thereby keeping the catalog up-to-date with the latest information. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the gun database with a few corrections and one new entry. The changes are mostly good, but I've found a couple of inconsistencies in the use of Roman numerals. Using standard Latin characters (like III and IV) instead of special Unicode characters for Roman numerals (Ⅲ and Ⅳ) would improve data consistency and adhere to the schema guidelines for the shortName field. Please see my specific comments for details.
| 10114,"marui","M4A1 CARBINE (EVOLT series)","","","M4A1 カービン (電動ガンエボルトシリーズ)","","",2,2,0,, | ||
| 10115,"marui","FPR MK4 (EVOLT RS series)","","","FPR MK4 (電動ガンエボルトRSシリーズ)","","",2,2,0,, | ||
| 10114,"marui","M4A1 CARBINE (EVOLT series)","","","M4A1カービン (電動ガンエボルトシリーズ)","","",2,2,0,, | ||
| 10115,"marui","FPR Mk IV (EVOLT RS series)","","","FPR Mk Ⅳ (電動ガンエボルトRSシリーズ)","","",2,2,0,, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency and to avoid special Unicode characters, it's better to use the standard Latin characters IV for the Roman numeral in the shortNameJa field. This aligns with the change you made in the shortName field on this same line.
10115,"marui","FPR Mk IV (EVOLT RS series)","","","FPR Mk IV (電動ガンエボルトRSシリーズ)","","",2,2,0,,
| 16022,"maruzen","CA870 TAC-10","","","CA870 タック10","","",4,1,0,, | ||
| 16023,"maruzen","CA870 TAC-10 CHARGER","","","CA870 タック10 チャージャー","","",4,1,0,, | ||
| 16024,"maruzen","CA870 TACTICAL Ⅱ","","","CA870 タクティカルⅡ","","",4,1,0,, | ||
| 16025,"maruzen","CA870 TACTICAL Ⅲ","","","CA870 タクティカルⅢ","","",4,1,0,, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shortName field should contain only Latin or Greek characters as per docs/GunCatalog.md. Please use III instead of the special Unicode character Ⅲ. For consistency, this should also be applied to the shortNameJa field. This change would also align with the correction made on line 1341 (MK4 to Mk IV).
16025,"maruzen","CA870 TACTICAL III","","","CA870 タクティカルIII","","",4,1,0,,
…nCatalog.csv