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

Trim suffix from wine version #1033

Merged
merged 3 commits into from
Dec 21, 2016
Merged

Trim suffix from wine version #1033

merged 3 commits into from
Dec 21, 2016

Conversation

cawa-93
Copy link
Contributor

@cawa-93 cawa-93 commented Dec 21, 2016

This is fix #1031 and #953 issue

@mention-bot
Copy link

@cawa-93, thanks for your PR! By analyzing the history of the files in this pull request, we identified @demetris-manikas, @seeekr and @ml1nk to be potential reviewers.

@@ -320,6 +320,11 @@ export async function checkWineVersion(checkPromise: Promise<string>) {
wineVersion = wineVersion.substring(0, spaceIndex)
}

const rcIndex = wineVersion.indexOf("-rc")
Copy link
Member

Choose a reason for hiding this comment

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

I think, we can just check - to be prepared for another suffixes.

Copy link
Contributor Author

@cawa-93 cawa-93 Dec 21, 2016

Choose a reason for hiding this comment

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

Then it may be simply makes sense to replace all - on .? In this case, you will see the difference between wine-2.0-rc1 and wine-2.0-rc2. They will be given to the form and 2.0.rc1 2.0.rc2. Semver be able to recognize this type of version?

Copy link
Member

Choose a reason for hiding this comment

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

may be simply makes sense to replace all "-" on "."?

It is not safe because we don't know format of rc — it can be not valid semver. So, it is better just to trim it.

you will see the difference between wine-2.0-rc1 and wine-2.0-rc2

Not required for now.

@cawa-93 cawa-93 changed the title Added check "RC" version Trim suffix from wine version Dec 21, 2016
@develar develar merged commit 090150c into electron-userland:master Dec 21, 2016
@develar
Copy link
Member

develar commented Dec 21, 2016

Thanks for PR!

gregnolle added a commit to voidbridge/electron-builder that referenced this pull request Dec 24, 2016
* commit 'dcf3dbb48979291fb91301610de9e417e0ab7b79': (43 commits)
  feat: LSTypeIsPackage for file associations
  fix: Trim suffix from wine version (electron-userland#1033)
  fix: Stub Executables missing in Squirrel.Windows 1.5.1
  fix: order of platform and arch npm env vars (electron-userland#1029)
  feat: Set platform npm environment variable
  feat: Use Electron.Net to send http requests for auto updater
  docs: typo (electron-userland#1026)
  refactor: reexport HttpError
  feat(appx): more customizable manifest fields
  chore: prefer const
  feat: 32 bit appx
  fix: yarn detection
  feat: update Squirrel.Windows to 1.5.1
  fix: update nsis to 3.0.1
  docs: Added details about setting AUMID for Windows 8/8.1 notifications.
  fix: update nsis to 3.0.1
  test: attempt to make Win CI green
  test: use yarn on Appveyor
  fix: ENOENT for symlinks because directory was not created
  fix(electron-auto-updater): load default provider only if custom was not set
  ...

# Conflicts:
#	src/cli/install-app-deps.ts
#	src/cli/node-gyp-rebuild.ts
#	src/packager.ts
#	src/publish/restApiRequest.ts
#	src/targets/nsis.ts
#	src/util/httpRequest.ts
#	src/yarn.ts
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.

TypeError: Invalid Version: 2.0-rc1.0
3 participants