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: set country in config, login to nvidia when starting #162
Conversation
5cf3f35
to
6a6197c
Compare
This can relatively easily be replicated across stores so that we don't end up with 10 copies of every store for each locale. I'm going to hold off on trying to implement this on other sites until this is tested and merged, but I like where your head is at on this. |
It'd be good to add a config based method for choosing location like in #163 (I closed the PR because it was very similar to this one and I like this one better) |
@hmellor was working on it. See most recent commit. |
93600e0
to
27dae92
Compare
Could you add GB from #161? Thanks |
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.
locales look good to me 👍
@jef ready for review and merge when you have a moment. Other stores with multiple regions should refactor to use |
@@ -19,6 +19,7 @@ SHOW_ONLY_SERIES="3080" | |||
SLACK_CHANNEL="SlackChannelName" | |||
SLACK_TOKEN="slack-token" | |||
STORES="bestbuy,bandh,nvidia" | |||
COUNTRY="usa" |
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.
Do we plan to allow multiple countries? e.g: I am in Canada, and would like to scan both US and CA stores
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.
Yes, this is the first step in localization. We don't want to localize every store all at once in case there are issues. Nvidia is the first and proof of concept, assuming all goes well we can work on implementing localization across stores.
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.
Work around would be to just run two instances with different configs, but also depends on the stores. IE FE's from nvidia would be the same stock
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.
Not a priority for me, but yes, this is the first step to getting there (while keeping the code clean-ish/manageable).
Description
Fixes #161. Refactors Nvidia to hide more of the logic. Adds support for a bunch of countries to Nvidia (with add to cart!) and support for 3090 (despite no DR IDSs).
Testing
Tested on
usa
andbelgium
. Use.env
asREADME
shows.