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

ProgramData version detection + Semver Version sorting #150

Closed

Conversation

QbDesu
Copy link
Contributor

@QbDesu QbDesu commented May 31, 2021

Fix proposal for #149
It's built on top of #139 so it's also a fix for #138. Which however still didn't explain why a lower version folder was sometimes detected, as there seemed to be a higher version folder detected.
This is also a bigger refactoring in general that also changes the validate functions which the original PR doesn't.

Tested to be working on Windows, but I did have to set it up the folders manually because discord installed itself in the normal appdata directory.

@QbDesu QbDesu force-pushed the programdata-path-search branch 2 times, most recently from e91d7bb to f4d21dc Compare June 1, 2021 00:45
@QbDesu
Copy link
Contributor Author

QbDesu commented Jun 2, 2021

Interesting thing that the current implementation in this PR does not account for:
a 1.0.9002 folder in %PROGRAMDATA%\%USERNAME%\Discord, so without the app- prefix, reported to contain nothing but a .first-run file.
Edit: actually it does account for it, but still an interesting observation.

@QbDesu QbDesu mentioned this pull request Jun 4, 2021
21 tasks
.eslintrc Outdated Show resolved Hide resolved
src/renderer/actions/paths/index.js Outdated Show resolved Hide resolved
src/renderer/actions/paths/linux.js Outdated Show resolved Hide resolved
src/renderer/actions/paths/linux.js Outdated Show resolved Hide resolved
src/renderer/actions/paths/linux.js Outdated Show resolved Hide resolved
src/renderer/actions/paths/mac.js Outdated Show resolved Hide resolved
@QbDesu QbDesu force-pushed the programdata-path-search branch from f4d21dc to 0726f78 Compare July 10, 2021 16:46
@QbDesu QbDesu changed the base branch from main to development July 10, 2021 16:47
@QbDesu
Copy link
Contributor Author

QbDesu commented Jul 10, 2021

I reworked the complete PR. Needs some testing on Windows but seems to work correctly on Linux.

@QbDesu QbDesu requested a review from zerebos July 10, 2021 16:49
@QbDesu
Copy link
Contributor Author

QbDesu commented Jul 10, 2021

Btw, I wonder when the first person called snap complains that they aren't supported.

@QbDesu QbDesu changed the title Version Folder Detection Refactoring ProgramData version detection + Semver Version sorting Jul 10, 2021
@dav1312
Copy link
Contributor

dav1312 commented Jul 19, 2021

Why is package-lock.json there? Isn't it generated already using npm i?

@QbDesu
Copy link
Contributor Author

QbDesu commented Jul 19, 2021

Yes, but the purpose of the package-lock.json is to record the versions of all installed dependencies, and the dependencies' dependencies, and so on. This is so that it can be reproduced. The package-lock.json will be used when running the npm ci which will try to repoduce the exact same package versions that are recorded in the file.

This is so that when a package gets an update that would break things or make them behave weirdly it won't mess up your build automation because it will use the same versions you used during development and for testing.

@zerebos
Copy link
Member

zerebos commented Jul 28, 2021

Technically package-lock.json should not be there as this project is using yarn and generates a yarn.lock file https://github.com/BetterDiscord/Installer/blob/main/yarn.lock

@QbDesu QbDesu force-pushed the programdata-path-search branch from 0726f78 to 3d415c8 Compare September 2, 2021 19:11
@QbDesu
Copy link
Contributor Author

QbDesu commented Sep 2, 2021

replaced the package-lock.json and updated yarn lockfile instead

@QbDesu QbDesu force-pushed the programdata-path-search branch from 3d415c8 to 024f1d7 Compare September 3, 2021 09:05
@QbDesu
Copy link
Contributor Author

QbDesu commented Sep 3, 2021

had to fix it because I accendentially commit some changes that were essentially undoing previous commits

@zerebos zerebos closed this in 384bef6 Oct 19, 2021
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