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

lint-helpers fetchLatestVersion compares each semver and returns latest #89

Closed
wants to merge 2 commits into from

Conversation

ThorstenHans
Copy link

As mentioned in #88 the fetchLatestVersion should compare all releases based on their version properties.

Copy link
Contributor

@kevinsawicki kevinsawicki left a comment

Choose a reason for hiding this comment

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

Thanks, let a few minor comments.

window.fetch('https://atom.io/download/atom-shell/index.json')
.then((response) => {
return response.json()
}).then((versions) => {
window.__devtron.latestElectronVersion = versions[0].version
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for catching this issue, I think it might be a simpler approach to use the https://www.npmjs.com/package/semver module for something like this, it has methods to get the greater version when given two versions.

@@ -1,6 +1,6 @@
{
"name": "devtron",
"version": "1.3.0",
"version": "1.3.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this change, this will be bumped when it is published.

@ThorstenHans
Copy link
Author

I would definitely prefer semver module but wasn't sure if it's okay to add another dependency, will update the PR during the weekend ;)

@ThorstenHans
Copy link
Author

@kevinsawicki changed the things you mentioned but looks like that https://atom.io/download/atom-shell/index.json is preventing XHR requests now ?? do you guys changed the webserver config and disallow CORS

@zeke
Copy link
Member

zeke commented Oct 10, 2016

It looks like that atom.io URL redirects to an S3 URL which doesn't allow cross-origin requests either:

Fetch API cannot load https://gh-contractor-zcbenz.s3.amazonaws.com/atom-shell/dist/index.json. No 'Access-Control-Allow-Origin' header is present on the requested resource.

That might mean the CORS configuration would need to be updated for both?

@ThorstenHans
Copy link
Author

@zeke I've tried both urls and none of them allows CORS. The spec says if it's a permanent request, the first endpoint has to allow the OPTIONS request and the second (which is the actual request) has to allow CORS for the GET request.

So it depends on how CORS is configured on the first endpoint. If OPTIONS requests were allowed, reconfiguring CORS to allow GET on the second should work.

@ThorstenHans ThorstenHans closed this by deleting the head repository Nov 4, 2022
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