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

fix(newegg): dedupe nitro+ models #1300

Merged
merged 5 commits into from Dec 12, 2020
Merged

fix(newegg): dedupe nitro+ models #1300

merged 5 commits into from Dec 12, 2020

Conversation

moiLegacy
Copy link
Contributor

Description

Fixes #1230
There were two Nitro+ models for newegg with same brand/model/series
Added a a new model type and updated the second Nitro+ in newegg

Testing

nada

RX6800XT was returning for negg when exceeding max price in .ENV

There were two identical Nitro models (brand,model,.series) but 2 different product numbers for newegg.  Added a new model called nitro+ SE (one of the products has PCI 4.0 SE interface)
@moiLegacy moiLegacy requested a review from jef as a code owner December 9, 2020 22:21
@moiLegacy
Copy link
Contributor Author

If your test is successful - similar approach might fix 1294 as well. I see for Amazon the same brand, model, series ....longer term fix might be to strip product number and include that in the max price check so that it doesn't just hit the first brand/model/series it finds in the dictionary/array

Copy link
Owner

@jef jef left a comment

Choose a reason for hiding this comment

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

Do you mind changing to nitro+ se? Otherwise, looks great! Thank you 😄

changed nitro+ SE to nitro+ se
@moiLegacy
Copy link
Contributor Author

updated as requested

Copy link
Owner

@jef jef left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you 😄

@jef jef changed the title fix: issue 1230 max price bug fix(newegg): dedupe nitro+ models Dec 12, 2020
@jef jef merged commit 7329c6e into jef:main Dec 12, 2020
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.

RX6800XT Stock on Newegg Generates Alert When Priced over MAX_PRICE_SERIES_RX6800XT Parameter
2 participants