[GitHub] latest release badge #1122

ritwickdey · 2017-10-02T20:55:14Z

pull request #905

Hi, please review the code. I've separated the code to lib/github-provider.js & made few modification on it.

[the branch was too old, I've managed conflicting code & removed few code with standard function]

Please guide me for further changes. [I have not yet added tests. I'll add it by tommrow & for now please ingore the lint issue. I'll fix it then.]

- Badge shows readable format of release date of latest version - Color fades with age of release

ritwickdey · 2017-10-02T21:01:01Z

lib/github-provider.js

+        const colorB = makeColorB('#007ec6', data ); // blue
+        const baseColor = Color(colorB);
+        const weeksSinceRelease = moment().diff(releaseDate, 'weeks');
+        const factor = Math.max(Math.min(weeksSinceRelease / 25, 0.9), 0.1);


Please don't ask me about this 😊😊😊....

Does the color scheme provided by age in color-formatters seem reasonable enough to use here, instead?

Okay, but that is not lighten or brighten color. Anyways, I am cleaning the existing code with age function

paulmelnikow

Thanks for picking this up!

paulmelnikow · 2017-10-02T21:16:12Z

lib/github-provider.js

+      try {
+        const data = JSON.parse(buffer);
+
+        if (!data.created_at) throw 'Project has no releases';


Instead of throwing, this should return a badge with a helpful message.

actually, problem is that data is always a object even if the the repo is the exist. So, data.created_at will not throw any error as it will be always null or a valid value.

If you think, we should return a valid message, then I've to check the status code first then data.created_at. -- NO PROBLEM AT ALL.

Should i do this?

Ah, I gotcha. Yes, it'd be better to add a test for a nonexistent repo, determine how to detect that condition, and return a useful message.

ok! Then, I should check the returned status code

paulmelnikow · 2017-10-02T21:17:16Z

lib/github-provider.js

+          sameDay : '[Today]',
+          lastWeek : '[last] dddd',
+          sameElse :  now => releaseDate.year() == now.year() ? 'D MMM' : 'D MMM YYYY'
+        });


Could you use formatDate from text-formatters?

Oh, the code it is already exists. 😃 ..

paulmelnikow · 2017-10-02T21:17:47Z

lib/github-provider.js

+        const colorB = makeColorB('#007ec6', data ); // blue
+        const baseColor = Color(colorB);
+        const weeksSinceRelease = moment().diff(releaseDate, 'weeks');
+        const factor = Math.max(Math.min(weeksSinceRelease / 25, 0.9), 0.1);


Does the color scheme provided by age in color-formatters seem reasonable enough to use here, instead?

paulmelnikow · 2017-10-02T21:18:10Z

server.js

@@ -3323,6 +3324,9 @@ cache(function(data, match, sendBadge, request) {
  });
 }));

+// GitHub release date integration.
+mapGithubReleaseDate(camp, githubApiUrl, githubAuth)


Could you add service tests?

Ah, just saw your note about that!

ritwickdey · 2017-10-03T07:19:11Z

lib/github-provider.js


 const { handleRequest: cache } = require('./request-handler');
 const {
  makeBadgeData: getBadgeData,
  makeLabel: getLabel,
-  getLogo
+  makeLogo: getLogo,


sorry, I made a mistake on previous pull request. I didn't noticed that getLogo is not defined in badge-data. It is alias of makeLogo

ritwickdey · 2017-10-03T13:59:27Z

lib/github-provider.js

+        //github return 404 if repo not found or no release
+        if(res.statusCode === 404) {
+          badgeData.text[1] = 'not found';
+          sendBadge(format, badgeData);


Is it good to return not found ? or just invalid??

Yea, no releases or repo not found would be clearer than invalid.

Big String ^^

It's helpful though! It tells the user what could be wrong.

ritwickdey · 2017-10-03T17:32:16Z

Hahaha Travis CI still in pending 🌞

paulmelnikow · 2017-10-03T18:50:51Z

I kicked the build. It's running now.

paulmelnikow · 2017-10-04T00:15:48Z

package.json

@@ -24,6 +24,7 @@
  "dependencies": {
    "camp": "~16.2.3",
    "chrome-web-store-item-property": "~1.1.2",
+    "color": "^1.0.3",


This is no longer needed.

👍 Oop! I forget to remove that.

paulmelnikow · 2017-10-04T00:16:36Z

service-tests/github.js

+t.create('Release Date. e.g release date|today')
+.get('/release-date/microsoft/vscode.json')
+.expectJSONTypes(Joi.object().keys({
+  name: Joi.equal('release date'),


You can simplify this and the one below to e.g. name: 'release date'. I've gone ahead and done with the existing tests in #1127.

paulmelnikow · 2017-10-04T00:17:10Z

service-tests/github.js

+.get('/release-date/not-valid-name/not-valid-repo.json')
+.expectJSONTypes(Joi.object().keys({
+  name: Joi.equal('release date'),
+  value: 'no releases or repo not found'


Since these are both hard-coded, it can be written like this:

.expectJSON({ name: 'release date', value: 'no releases or repo not found' })

ritwickdey · 2017-10-04T04:54:32Z

Not ready yet.

ritwickdey · 2017-10-04T05:20:33Z

ready.

tooomm · 2017-10-04T08:24:22Z

Will this account for any release?
Or just "full" releases (not pre-releases)?

ritwickdey · 2017-10-04T10:16:15Z

hi, @tooomm full release

tooomm · 2017-10-04T10:25:06Z

Does it makes sense to extend this for
https://img.shields.io/github/prerelease-date/SubtitleEdit/subtitleedit.svg

Many repos use pre-releases to distribute betas next to stable releases.
It would be handy to list a date badge for both independently.

ritwickdey · 2017-10-04T10:36:22Z

yep! right! okay I'll add. Lets wait for what @paulmelnikow says - should I open a different pull request on it or it will be included here? (I am saying because this PR is extended from #905)

Btw, I think, there is no explicit API for getting latest pre-release version.

tooomm · 2017-10-04T10:48:41Z

Btw, I think, there is no explicit API for getting latest pre-release version.

You might be right there. For displaying releases it works like that:

GitHub release: | https://img.shields.io/github/release/qubyte/rubidium.svg
GitHub (pre-)release: | https://img.shields.io/github/release/qubyte/rubidium/all.svg

So it looks like one just displays all releases (including pre-releases) and the first just displays full releases.
As long as there is no newer pre-release both show the same - as in the example on the shields page.

Example with a different pre-release as seen in a readme on github:

paulmelnikow · 2017-10-04T14:20:35Z

Github service tests passing locally.

That's a great idea to add prerelease-date. Let's do it in a separate PR so we can close this out!

Thanks for all your work on this! 🏅

tooomm · 2017-10-04T16:17:40Z

If you keep on looking into that prerelease date, please @ link me when you open the PR.
I happily get informed about any progress! 👍

ritwickdey · 2017-10-05T07:38:06Z

Hi @tooomm , #1133

agboom and others added 4 commits March 27, 2017 20:54

Add Github release date integration

b7e75a7

- Badge shows readable format of release date of latest version - Color fades with age of release

Resolved merge conflict

6e37517

fixed breaking logo on last commit

5e2fa13

code separated & few simplification

2d54ae4

ritwickdey commented Oct 2, 2017

View reviewed changes

paulmelnikow mentioned this pull request Oct 2, 2017

Add Github release date integration #905

Closed

paulmelnikow reviewed Oct 2, 2017

View reviewed changes

paulmelnikow added the service-badge New or updated service badge label Oct 2, 2017

ritwickdey added 2 commits October 3, 2017 12:12

code cleaned

c09aa83

test added

52cbb8a

ritwickdey commented Oct 3, 2017

View reviewed changes

throw statement removed & one extra test added

1914656

ritwickdey commented Oct 3, 2017

View reviewed changes

ritwickdey added 2 commits October 3, 2017 19:32

lint

92f12b4

rename

0968b0d

paulmelnikow reviewed Oct 4, 2017

View reviewed changes

ritwickdey added 2 commits October 4, 2017 10:06

unnecessary package removed

618c675

test simplified

31e58c0

ritwickdey force-pushed the github-release branch from 20306b5 to 31e58c0 Compare October 4, 2017 04:49

html added

d727231

ritwickdey changed the title ~~[GitHub] last release badge~~ [GitHub] latest release badge Oct 4, 2017

paulmelnikow approved these changes Oct 4, 2017

View reviewed changes

paulmelnikow merged commit e7f1f57 into badges:master Oct 4, 2017

ritwickdey mentioned this pull request Oct 5, 2017

[GitHub] badge for (pre-)release date #1133

Merged

dependabot-preview bot mentioned this pull request Aug 2, 2018

Bump eslint-plugin-import from 2.9.0 to 2.13.0 #1836

Merged

This was referenced Aug 21, 2023

[Snyk] Upgrade eslint-plugin-jsdoc from 39.3.3 to 46.4.5 Woodpile37/shields#24

Merged

[Snyk] Upgrade eslint-plugin-jsdoc from 39.3.3 to 46.4.6 Woodpile37/shields#49

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GitHub] latest release badge #1122

[GitHub] latest release badge #1122

ritwickdey commented Oct 2, 2017

ritwickdey Oct 2, 2017

paulmelnikow Oct 2, 2017

ritwickdey Oct 3, 2017

paulmelnikow left a comment

paulmelnikow Oct 2, 2017

ritwickdey Oct 3, 2017

paulmelnikow Oct 3, 2017

ritwickdey Oct 3, 2017

paulmelnikow Oct 2, 2017

ritwickdey Oct 3, 2017

paulmelnikow Oct 2, 2017

paulmelnikow Oct 2, 2017

paulmelnikow Oct 2, 2017

ritwickdey Oct 3, 2017

ritwickdey Oct 3, 2017

paulmelnikow Oct 3, 2017

ritwickdey Oct 3, 2017

paulmelnikow Oct 3, 2017

ritwickdey commented Oct 3, 2017

paulmelnikow commented Oct 3, 2017

paulmelnikow Oct 4, 2017

ritwickdey Oct 4, 2017

paulmelnikow Oct 4, 2017

paulmelnikow Oct 4, 2017

ritwickdey commented Oct 4, 2017

ritwickdey commented Oct 4, 2017

tooomm commented Oct 4, 2017

ritwickdey commented Oct 4, 2017

tooomm commented Oct 4, 2017

ritwickdey commented Oct 4, 2017

tooomm commented Oct 4, 2017

paulmelnikow commented Oct 4, 2017

tooomm commented Oct 4, 2017

ritwickdey commented Oct 5, 2017

[GitHub] latest release badge #1122

[GitHub] latest release badge #1122

Conversation

ritwickdey commented Oct 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulmelnikow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ritwickdey commented Oct 3, 2017

paulmelnikow commented Oct 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ritwickdey commented Oct 4, 2017

ritwickdey commented Oct 4, 2017

tooomm commented Oct 4, 2017

ritwickdey commented Oct 4, 2017

tooomm commented Oct 4, 2017

ritwickdey commented Oct 4, 2017

tooomm commented Oct 4, 2017

paulmelnikow commented Oct 4, 2017

tooomm commented Oct 4, 2017

ritwickdey commented Oct 5, 2017