Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

T3chguy/settings versions improvements #857

Merged
merged 5 commits into from
May 4, 2017
Merged

T3chguy/settings versions improvements #857

merged 5 commits into from
May 4, 2017

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented May 2, 2017

Less naive semver detection
now links to trailing commit instead of to latest tag if there is one
removes redundant 'v's

soft-depends on element-hq/element-web#3683

removed redundant v prefix (key already says version)
links to most applicable version/tag
tag-commit -> commit
commit1-commit2-commit3 -> commit1
(v)x.y.z -> tag<x.y.z>
commit -> commit

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@@ -530,7 +538,7 @@ module.exports = React.createClass({
},

_renderUserInterfaceSettings: function() {
var client = MatrixClientPeg.get();
// const client = MatrixClientPeg.get();
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks unused, I'd remove it entirely.

<li><label>Device ID:</label> <span><code>{deviceId}</code></span></li>
<li><label>Device key:</label> <span><code><b>{identityKey}</b></code></span></li>
<li><label>Device ID:</label> <span><code>{deviceId}</code></span></li>
<li><label>Device key:</label> <span><code><b>{identityKey}</b></code></span></li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely sure why more spaces are needed here

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I didn't intend to change that, IDE must have thought it was being clever

const uriTail = (token.startsWith('v') && token.includes('.')) ? `releases/tag/${token}` : `commit/${token}`;
return `https://github.com/${repo}/${uriTail}`;
}
const semVerRegex = /^v?(\d+\.\d+\.\d+)(?:-(?:\d+-g)?(.+))?|$/i;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should probably more closely match https://github.com/mojombo/semver/blob/master/semver.svg so (\d+\.\d+\.\d+) seems OK but the pre-release part (?:-(?:\d+-g)?(.+)) looks like it could be a bit more flexible (allowing more than just "-g" unless I've misunderstood something).

Copy link
Member Author

Choose a reason for hiding this comment

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

the part you mentioned is actually for catching the trailing commit (which is newer than the base tag) Have now updated the regex to catch rc's and also -dirty git describes.
Regex and examples: https://regex101.com/r/Wl3CqO/4/

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@lukebarnard1 lukebarnard1 merged commit 7918ff2 into matrix-org:develop May 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants