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

Add Script.installsSinceUpdate & show in author panel. #333

Merged
merged 1 commit into from
Sep 2, 2014

Conversation

Zren
Copy link
Contributor

@Zren Zren commented Aug 28, 2014

This only tracks the current version right now.

This is one method of creating a useful install count. We could expand on this by adding another property with { type: Object } that contains { version: installCount, version: installCount }.

Only shown in the author panel for now.

@cletusc
Copy link
Contributor

cletusc commented Aug 28, 2014

👍 This is similar to what I was trying to explain when I created this google group thread.

@Martii
Copy link
Member

Martii commented Aug 28, 2014

+1 for sure here... really difficult to do some admin duties and this is a step in the right direction for that. I ran into a situation within the last few days that I couldn't tell what script came first too. e.g. author wise from two authors. I have yet to find a "signedup date" for the author... so my only remedy currently is to flag them both and let them sort it out. :\

@Zren Zren added DB and removed needs discussion labels Aug 28, 2014
@Martii
Copy link
Member

Martii commented Aug 28, 2014

Btw is installs versus the version backwards? I noticed on absent @version too it shows "Current" which is very kewl. :)

@Zren
Copy link
Contributor Author

Zren commented Aug 28, 2014

Er... it was Installs per Version before I shortened it to a slash. I guess that might be confusing. I could tack on "installs" at the end. Eg: [v2] 234 installs

@Martii
Copy link
Member

Martii commented Aug 28, 2014

I could tack on "installs" at the end. Eg: [v2] 234 installs

Would probably be better for translations too.

@Zren
Copy link
Contributor Author

Zren commented Aug 28, 2014

Made it a label too.

@jerone
Copy link
Contributor

jerone commented Aug 28, 2014

Wouldn't it be more useful to have some sort of history table (including e.g. source changes).
After each update the installs count since {date} will be reset and forgotten.

@vBm
Copy link
Contributor

vBm commented Aug 28, 2014

👍 Been wanting this for so long :)

@Martii
Copy link
Member

Martii commented Aug 28, 2014

(including e.g. source changes).

This portion needs to be team biz as well as someone needs to assign them self to it as "chair person". Sizzle has already stated that we ~"may have changelogs" but it's not the first and foremost on his mind nor mine. e.g. commit history may be a MIGHTFIX.

@Zren Zren added the PR READY label Sep 1, 2014
@Zren
Copy link
Contributor Author

Zren commented Sep 1, 2014

Wouldn't it be more useful to have some sort of history table (including e.g. source changes).
After each update the installs count since {date} will be reset and forgotten.

Yes, but that's a lot more work than 10 lines of code that this entails.

Added PR Ready for these both my PRs.

@Martii
Copy link
Member

Martii commented Sep 2, 2014

Er... it was Installs per Version before I shortened it to a slash. I guess that might be confusing.

Still seems like it is going to confuse those new to the site EDIT: perhaps expecting a ratio or percent. You say "Installs / Versions" (assume you mean "per" here) but underneath it is shows "Version X installs".

@jerone
Copy link
Contributor

jerone commented Sep 2, 2014

@Martii commented on 2 sep. 2014 05:59 CEST:

Er... it was Installs per Version before I shortened it to a slash. I guess that might be confusing.

Still seems like it is going to confuse those new to the site EDIT: perhaps expecting a ratio or percent. You say "Installs / Versions" (assume you mean "per" here) but underneath it is shows "Version X installs".

I agree. My preference goes to using the following as it's not an mathematical formula:

Installs per version

Needs to change the "since" date too when going live:

since 27 Aug 2014

@Martii
Copy link
Member

Martii commented Sep 2, 2014

Needs to change the "since" date too when going live

I think moment can be used to format it to however we want it for each locale eventually... but you are right. I'm translating the dev page with a few translators just to see how it "looks"... I don't know every foreign language but it may translate back and forth better in clearer sentence type structures.

@jerone
Copy link
Contributor

jerone commented Sep 2, 2014

@Martii commented on 2 sep. 2014 10:13 CEST:

Needs to change the "since" date too when going live

I think moment can be used to format it to however we want it for each locale eventually... but you are right. I'm translating the dev page with a few translators just to see how it "looks"... I don't know every foreign language but it may translate back and forth better in clearer sentence type structures.

Using Moment.js here sounds like a good solution. Later, when we implement i18n (#18), we can add a locale key as the third parameter.

@Zren
Copy link
Contributor Author

Zren commented Sep 2, 2014

It was in both of the screenshots, but whatever, may as well since I'm updating the date anyways.

Moment is used for formatting Date objects, which we're not doing. If i18n ever gets implemented, treat it as a regular string you need to translate (if the warning is even still needed in that distant future).

@Zren
Copy link
Contributor Author

Zren commented Sep 2, 2014

2 Sept 2014 & per done.

@jerone
Copy link
Contributor

jerone commented Sep 2, 2014

@Zren commented on 2 sep. 2014 11:04 CEST:

2 Sept 2014 & per done.

Don't forget the PR READY label 😄

@Martii
Copy link
Member

Martii commented Sep 2, 2014

LOL @jerone We're all here right now and checking this out so I can make the exception this one time but yes it should be done. I just tested it and it looks good.

@Zren Zren added the PR READY label Sep 2, 2014
Martii added a commit that referenced this pull request Sep 2, 2014
Add Script.installsSinceUpdate & show in author panel.
@Martii Martii merged commit b617f46 into OpenUserJS:master Sep 2, 2014
@Martii Martii removed the PR READY label Sep 2, 2014
@Martii
Copy link
Member

Martii commented Sep 2, 2014

btw did anyone notice GH seems to have made a change on cumulative changes in a pr to only show one commit now? e.g. Zrens commits (all 3 of them for this pr) are only referenced once now... at least here.

@Zren
Copy link
Contributor Author

Zren commented Sep 2, 2014

That's because I did git reset --soft HEAD^1 then redid the commit then git push origin installssinceupdate -f.

This way, only one commit actually gets merged.

@Martii
Copy link
Member

Martii commented Sep 2, 2014

Interesting. That might keep things tidy but terrible for teaching... and referencing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DB Pertains inclusively to the Database operations. feature Something we don't already have implemented to the best of knowledge but would like to see.
Development

Successfully merging this pull request may close these issues.

5 participants