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

Handle .x version syntax with latest release #13

Merged
merged 9 commits into from
Aug 19, 2019
Merged

Handle .x version syntax with latest release #13

merged 9 commits into from
Aug 19, 2019

Conversation

subosito
Copy link
Contributor

This PR adds the ability to resolve .x version syntax to the latest release, e.g.: 1.10.x as 1.10.8.

This PR is a solution for #11.

Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution - generally this looks great, just left a couple of comments!

//append patch version if not available
return version.concat('.0');
}
return version;
}

async function determineVersion(version: string): Promise<string> {
if (!version.endsWith('.x')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we do something similar to normalizeVersion here, but instead of appending 0s at the end, append a .x? What I'm after is, I want people to just be able to specify 1.10 and have that resolve to 1.10.x, since referring to versions like that is pretty common.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I've pushed an update to handle this.

src/installer.ts Outdated Show resolved Hide resolved
const versions = await getAvailableVersions();
const possibleVersions = versions.filter(v => v.startsWith(version));

const versionMap = new Map();
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than creating a map here, can you just do something like return possibleVersions.sort() (an alphabetic sort which matches the semver versioning ordering). I think what you're doing here is basically just doing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I am doing here is sorting using semver, which different with regular sort function.

sort returns:

[ '1.10',
  '1.10.1',
  '1.10.2',
  '1.10.3',
  '1.10.4',
  '1.10.5',
  '1.10.6',
  '1.10.7',
  '1.10.8',
  '1.10beta1',
  '1.10beta2',
  '1.10rc1',
  '1.10rc2' ]

While this returns:

[ '1.10.8',
  '1.10.7',
  '1.10.6',
  '1.10.5',
  '1.10.4',
  '1.10.3',
  '1.10.2',
  '1.10.1',
  '1.10',
  '1.10rc2',
  '1.10rc1',
  '1.10beta2',
  '1.10beta1' ]

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh good point, sorry my mistake

package-lock.json Outdated Show resolved Hide resolved
@damccorm
Copy link
Contributor

This looks great - thanks for doing this!

@damccorm damccorm merged commit 632d18f into actions:master Aug 19, 2019
koenrh added a commit to koenrh/s3enum that referenced this pull request Aug 19, 2019
This will make sure that it will always test against the latest patch
version. actions/setup-go#13
@tie tie mentioned this pull request Dec 18, 2019

core.debug(`evaluating ${versions.length} versions`);

if (version.length === 0) {
Copy link

Choose a reason for hiding this comment

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

versions.length.

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.

None yet

3 participants