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

[elm-package] Fix elm package service #1986

Merged
merged 5 commits into from
Aug 28, 2018
Merged

[elm-package] Fix elm package service #1986

merged 5 commits into from
Aug 28, 2018

Conversation

j-maas
Copy link
Contributor

@j-maas j-maas commented Aug 27, 2018

With the release of Elm 0.19.0, the package file changed (from elm-package.json to elm.json), which breaks the badge.

(This replaces #1979 due to a refactor.)

@shields-ci
Copy link

shields-ci commented Aug 27, 2018

Messages
📖

✨ Thanks for your contribution to Shields, @y0hy0h!

Generated by 🚫 dangerJS

@@ -12,8 +12,8 @@ module.exports = class ElmPackage extends LegacyService {
cache((data, match, sendBadge, request) => {
const urlPrefix = 'http://package.elm-lang.org/packages'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to use the https version here?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, if that is supported, that would be great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The https URL is online, but I don't know if shields handles it. I guess I'll just change this, since I have to trigger the tests anyway.

@paulmelnikow paulmelnikow changed the title [elm-package] Fix elm package service [ElmPackage] Fix elm package service Aug 27, 2018
@paulmelnikow
Copy link
Member

paulmelnikow commented Aug 27, 2018

The CI failed initially because I told you the wrong tag. The format just changed and I'm still getting the hang of it!

Added: Despite the green check mark, the service tests have not run. (Ref #1936) @y0hy0h If you push a new commit with the scheme change, hopefully they will run then. 🤞

@j-maas
Copy link
Contributor Author

j-maas commented Aug 27, 2018

There still seems to be an issue with the service name.

@paulmelnikow
Copy link
Member

Argh, I guess I had that title format right to begin with.

When I run the tests locally using npm run test:services -- --only=elm-package, I get these two failures. It looks like the tests need to be updated to reflect the formatting change.


  1) ELM PACKAGE
       gets the package version of elm-lang/core

	[ GET /v/elm-lang/core.json ]:
     ValidationError: child "name" fails because ["name" must be one of [elm-package]]
      at Object.exports.process (node_modules/joi/lib/errors.js:196:19)
      at internals.Object._validateWithOptions (node_modules/joi/lib/types/any/index.js:675:31)
      at module.exports.internals.Any.root.validate (node_modules/joi/lib/index.js:138:23)
      at Object.pathMatch.matchJSONTypes (node_modules/icedfrisby/lib/pathMatch.js:303:9)
      at _expect (node_modules/icedfrisby/lib/icedfrisby.js:563:10)
      at IcedFrisbyNock._invokeExpects (node_modules/icedfrisby/lib/icedfrisby.js:1261:26)
      at start (node_modules/icedfrisby/lib/icedfrisby.js:1244:12)
      at Request.runCallback [as _callback] (node_modules/icedfrisby/lib/icedfrisby.js:1131:16)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:442:20)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:442:20)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:442:20)
      at endReadableNT (_stream_readable.js:1081:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  2) ELM PACKAGE
       invalid package name

	[ GET /v/elm-community/frodo-is-not-a-package.json ]:

      AssertionError: expected { Object (name, value) } to deeply equal { Object (name, value) }
      + expected - actual

       {
      -  "name": "elm package"
      +  "name": "elm-package"
         "value": "invalid"
       }

      at Object.pathMatch.matchJSON (node_modules/icedfrisby/lib/pathMatch.js:138:38)
      at _expect (node_modules/icedfrisby/lib/icedfrisby.js:583:10)
      at IcedFrisbyNock._invokeExpects (node_modules/icedfrisby/lib/icedfrisby.js:1261:26)
      at start (node_modules/icedfrisby/lib/icedfrisby.js:1244:12)
      at Request.runCallback [as _callback] (node_modules/icedfrisby/lib/icedfrisby.js:1131:16)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:442:20)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:442:20)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:442:20)
      at endReadableNT (_stream_readable.js:1081:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

@paulmelnikow paulmelnikow changed the title [ElmPackage] Fix elm package service [elm-package] Fix elm package service Aug 27, 2018
@paulmelnikow paulmelnikow added bug Bugs in badges and the frontend service-badge Accepted and actionable changes, features, and bugs labels Aug 27, 2018
@j-maas j-maas changed the title [elm-package] Fix elm package service [elm package] Fix elm package service Aug 28, 2018
@j-maas j-maas changed the title [elm package] Fix elm package service [elm-package] Fix elm package service Aug 28, 2018
@j-maas
Copy link
Contributor Author

j-maas commented Aug 28, 2018

The tests are passing now. However, I'm not entirely sure on the following naming inconsistency:

The service is called elm-package, but the badge will display elm package (and internally some fields will be named the latter). I don't know if it is even possible to rename the service to elm package (because of the whitespace).
I guess this depends on whether there is a guideline for this sort of naming issue. I personally think it's ok, but it's not perfect.

Is there a simple fix to this I'm not seeing?

@paulmelnikow
Copy link
Member

The ID is only used for running the test. This looks great! Thank you!

@paulmelnikow paulmelnikow merged commit edea36d into badges:master Aug 28, 2018
@shields-deployment
Copy link

This pull request was merged to master branch. Now this change is waiting for deployment.
Deploys usually happen every few weeks. After deployment changes are copied to gh-pages branch.

This badge displays deployment status:

@j-maas j-maas deleted the patch-2 branch August 28, 2018 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs in badges and the frontend service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants