-
Notifications
You must be signed in to change notification settings - Fork 0
Store old index for breach before reindexing #17
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
Assign old index to breach before updating the index.
Summary of ChangesHello @alphaleadership, 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 modifies the breach reindexing script to ensure that the original index of each breach is preserved. By introducing an Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 correctly implements the logic to store the old index of a breach before re-indexing. My review includes a minor suggestion to improve code style for better readability and consistency with JavaScript best practices.
| if(!breach.IsRetired){ | ||
| i++ | ||
| } | ||
| breach.oldindex=breach.index |
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 better readability and to avoid potential issues with Automatic Semicolon Insertion (ASI), it's good practice to add spaces around operators and end statements with a semicolon.
Regarding the property name, you might consider using camelCase (oldIndex) to align with common JavaScript conventions, though I see the existing index property is lowercase, so oldindex maintains local consistency.
| breach.oldindex=breach.index | |
| breach.oldindex = breach.index; |
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.
1 issue found across 1 file
Confidence score: 5/5
- Minor style nits in
scripts/resort-breaches.js(camelCase and spacing) are low-risk and don’t affect runtime behavior. - Pay close attention to
scripts/resort-breaches.js- alignoldIndexcasing and spacing for consistency.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="scripts/resort-breaches.js">
<violation number="1" location="scripts/resort-breaches.js:75">
P3: Property name `oldindex` should use camelCase (`oldIndex`) for consistency with other properties like `isNSFW`. Also, add spaces around `=` to match the style of the adjacent line `breach.index = i;`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if(!breach.IsRetired){ | ||
| i++ | ||
| } | ||
| breach.oldindex=breach.index |
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.
P3: Property name oldindex should use camelCase (oldIndex) for consistency with other properties like isNSFW. Also, add spaces around = to match the style of the adjacent line breach.index = i;.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/resort-breaches.js, line 75:
<comment>Property name `oldindex` should use camelCase (`oldIndex`) for consistency with other properties like `isNSFW`. Also, add spaces around `=` to match the style of the adjacent line `breach.index = i;`.</comment>
<file context>
@@ -72,6 +72,7 @@ db.breaches.forEach((breach, idx) => {
if(!breach.IsRetired){
i++
}
+ breach.oldindex=breach.index
breach.index = i;
})
</file context>
| breach.oldindex=breach.index | |
| breach.oldIndex = breach.index |
Assign old index to breach before updating the index.
Summary by cubic
Store the previous index on each breach as oldindex before reindexing so we can track changes and update references after resorting. This prevents losing the original index when retired breaches are skipped.
Written for commit aca675f. Summary will update on new commits.