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: Add More 50xx series, and add copy paste templates for adding more links #3191

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Mr-Tech-13
Copy link
Contributor

As stated in the title I added 50 series links for Bestbuy, Newegg, Pny, and Zotac. I also added some AMD CPUs. I also made copy paste templates at the bottom so it is easier to bulk add cards. The biggest thing was adding all of the Ubiquiti Switches, Cameras, Wifi, and Firewalls.

Enjoy!

@Mr-Tech-13 Mr-Tech-13 requested a review from jef as a code owner January 12, 2025 18:16
@JDIacobbo
Copy link

JDIacobbo commented Jan 16, 2025

I attempted to build this from the docker file to test it (read: run it early) and I am getting the following error when building the docker image:

[+] Building 8.5s (16/21)                                        docker:default
 => [internal] load build definition from Dockerfile                       0.0s
 => => transferring dockerfile: 1.10kB                                     0.0s
 => [internal] load metadata for docker.io/library/node:16.18.0-alpine3.1  0.2s
 => [internal] load .dockerignore                                          0.0s
 => => transferring context: 94B                                           0.0s
 => [internal] load build context                                          0.0s
 => => transferring context: 767.79kB                                      0.0s
 => [builder  1/10] FROM docker.io/library/node:16.18.0-alpine3.16@sha256  0.0s
 => CACHED [stage-1 2/8] RUN apk add --no-cache chromium                   0.0s
 => CACHED [stage-1 3/8] RUN addgroup -S appuser && adduser -S -g appuser  0.0s
 => CACHED [stage-1 4/8] WORKDIR /app                                      0.0s
 => CACHED [builder  2/10] WORKDIR /build                                  0.0s
 => CACHED [builder  3/10] COPY package.json package.json                  0.0s
 => CACHED [builder  4/10] COPY package-lock.json package-lock.json        0.0s
 => CACHED [builder  5/10] COPY tsconfig.json tsconfig.json                0.0s
 => CACHED [builder  6/10] RUN npm ci                                      0.0s
 => CACHED [builder  7/10] COPY src/ src/                                  0.0s
 => CACHED [builder  8/10] COPY test/ test/                                0.0s
 => ERROR [builder  9/10] RUN npm run compile                              8.2s
------                                                                          
 > [builder  9/10] RUN npm run compile:                                         
0.580                                                                           
0.580 > [email protected] compile
0.580 > tsc
0.580 
7.821 src/store/lookup.ts:501:51 - error TS2551: Property 'ryzen7800x3d' does not exist on type '{ 3050: number; 3060: number; '3060ti': number; 3070: number; '3070ti': number; 3080: number; '3080ti': number; 3090: number; '4080-12g': number; '4080-16g': number; 4090: number; 5070: number; '5070ti': number; 5080: number; ... 26 more ...; xboxsx: number; }'. Did you mean 'ryzen9800x3d'?
7.821 
7.821 501     const maxPrice = config.store.maxPrice.series[link.series];
7.821                                                       ~~~~~~~~~~~
7.821 
7.822 
7.822 Found 1 error.
7.822 
------
Dockerfile:18
--------------------
  16 |     COPY src/ src/
  17 |     COPY test/ test/
  18 | >>> RUN npm run compile
  19 |     RUN npm prune --production
  20 |     
--------------------
ERROR: failed to solve: process "/bin/sh -c npm run compile" did not complete successfully: exit code: 1

I have no issue building from jef's main branch.

@Mr-Tech-13
Copy link
Contributor Author

@JDIacobbo So sorry I mistyped something in the code.. I will update that right now

@Mr-Tech-13
Copy link
Contributor Author

Try Now? I may need to revamp my ubiquity changes

@JDIacobbo
Copy link

Still won't compile with the same error.

jef
jef previously approved these changes Jan 17, 2025
@jef
Copy link
Owner

jef commented Jan 17, 2025

Another absolute banger!!! Thank you 🙏 😍

@jef jef changed the title Add More 50xx series, All Ubiquiti Wifi, Camera, Switching, and add copy paste templates for adding more links feat: Add More 50xx series, All Ubiquiti Wifi, Camera, Switching, and add copy paste templates for adding more links Jan 17, 2025
@jef
Copy link
Owner

jef commented Jan 17, 2025

Looks like @JDIacobbo is right. Let me know if you need any help!

@@ -39,6 +39,12 @@ DISCORD_NOTIFY_GROUP_RYZEN5600=
DISCORD_NOTIFY_GROUP_RYZEN5800=
DISCORD_NOTIFY_GROUP_RYZEN5900=
DISCORD_NOTIFY_GROUP_RYZEN5950=
DISCORD_NOTIFY_GROUP_RYZEN7800X3D=
DISCORD_NOTIFY_GROUP_RYZEN9800X3D=
DISCORD_NOTIFY_GROUP_RYZEN9600=
Copy link
Owner

Choose a reason for hiding this comment

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

Also can I make a nit about these? Are the models 9600 or 9600X? I think the same applies for the other models.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good catch they are x variants, not non x. I'll fix that when I'm at my desk next

'https://secure.newegg.com/Shopping/AddtoCart.aspx?Submit=ADD&ItemList=N82E16814137919',
model: 'gaming trio oc',
series: '5090',
url: 'https://www.newegg.com/pN82E16814137919',
Copy link

@JDIacobbo JDIacobbo Jan 18, 2025

Choose a reason for hiding this comment

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

There is a typo in this line (1647). You are missing a / after the p.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@Mr-Tech-13
Copy link
Contributor Author

@jef ok so I remove all my UI stuff because of a bug I found. I will make that a separate PR. However regarding the discrepancy with the 9600 vs non x version, I dont think the 5000 series differentiated the two which is why I did it the way I did. It should be good now

@Mr-Tech-13
Copy link
Contributor Author

@JDIacobbo I found and fixed the bug. see this

@Mr-Tech-13 Mr-Tech-13 changed the title feat: Add More 50xx series, All Ubiquiti Wifi, Camera, Switching, and add copy paste templates for adding more links feat: Add More 50xx series, and add copy paste templates for adding more links Jan 18, 2025
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.

3 participants