US-190 avec ou sans internet #217

bdavidxyz · 2016-12-21T15:47:15Z

No description provided.

coveralls · 2016-12-21T15:59:35Z

Coverage decreased (-0.5%) to 83.005% when pulling f300af4 on 190-no-internet into 6420662 on dev.

jbuget · 2016-12-21T15:59:53Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2016-12-21T16:16:31Z

Coverage decreased (-0.5%) to 83.005% when pulling 5ca5acf on 190-no-internet into 6420662 on dev.

jbuget · 2016-12-21T16:16:48Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2016-12-21T16:23:08Z

Coverage decreased (-0.5%) to 83.005% when pulling 447e17f on 190-no-internet into 6420662 on dev.

jbuget · 2016-12-21T16:23:25Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2016-12-26T11:36:20Z

Coverage increased (+9.6%) to 93.08% when pulling ca0fa3e on 190-no-internet into 6420662 on dev.

jbuget · 2016-12-26T11:36:38Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2016-12-28T01:50:24Z

Coverage increased (+9.3%) to 92.857% when pulling 7d91c40 on 190-no-internet into 6420662 on dev.

jbuget · 2016-12-28T01:50:38Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

jbuget

Quelques interrogations et des micro-modifs en mode "easy".

Il y a aussi plein de commentaires – je suis pas fan car tout code en commentaire est un gros risque de code obsolète avant / sans même qu'on s'en rende compte – mais j'ai pas envie de bloquer l'US pour ça.

jbuget · 2017-01-03T14:14:31Z

api/.eslintrc

@@ -1 +1,4 @@
 extends: '../.eslintrc'
+
+globals:
+  include: true


Pourquoi est-on obligé d'autoriser le mot include ? question vraiment naïve : je ne connais pas la différence avec require et j'imagine que ESLint a une raison pour ne pas l'accepter de base.

Avec la modif de api/lib/settings.js je vois pour quoi on l'ajoute. Par contre, j'ai une remarque sur cette ligne, cf. ci-dessous.

jbuget · 2017-01-03T14:17:54Z

api/lib/settings.js

+global.abs_path = function(path) {
+  return global.base_dir + path;
+};
+global.include = function(file) {


Dans quel cas faut-il utiliser la méthode custom include plutôt que l'import standard require ?

jbuget · 2017-01-03T14:19:35Z

api/tests/acceptance/application/challenge-controller_test.js

+        expect(challenge.attributes.instruction).to.equal('Que peut-on dire des œufs de catégorie A ?\n');
+        expect(challenge.attributes.proposals).to.equal('- Ils sont bio.\n- Ils pèsent plus de 63 grammes.\n- Ce sont des oeufs frais.\n- Ils sont destinés aux consommateurs.\n- Ils ne sont pas lavés.\n');
+        expect(challenge.attributes.type).to.equal('QCM');
+        expect(challenge.attributes['has-internet-and-tools']).to.equal(true);
        done();
      });


Merci pour la transformation :-)

jbuget · 2017-01-03T14:22:21Z

api/tests/unit/utils/lodash-utils_test.js

@@ -48,6 +48,33 @@ describe('Unit | Utils | lodash-utils', function () {
  });


+  describe('areCSVequivalent', function () {
+    it('when no arg are given, should return false', function (done) {


Une question que je me pose, est-ce qu'on décide qu'on se force à utiliser done même quand on n'est pas dans un contexte asynchrone ? Dans ce cas et ceux qui suivent, par exemple, on fait un simple expect tout ce qu'il y a de plus synchrone. On peut peut-être s'économiser des lignes et de l'énergie en ommettant done, non ? WDYT ?

si pas besoin de done(), on le supprime

jbuget · 2017-01-03T14:23:04Z

live/app/models/answer/value-as-array-of-boolean-mixin.js

@@ -1,26 +1,14 @@
 import Ember from 'ember';
-import _ from 'lodash/lodash';
+// import _ from 'pix-live/utils/lodash-custom';


jbuget · 2017-01-03T14:25:35Z

live/package.json

@@ -64,7 +63,8 @@
    "replace": "^0.3.0",
    "stylelint": "^7.2.0",
    "stylelint-config-standard": "^13.0.0",
-    "tap-xunit": "^1.4.0"
+    "tap-xunit": "^1.4.0",
+    "phantomjs-prebuilt":"2.1.14"


Du coup, il faut modifier le README à la racine pour supprimer dans la partie "installation" le prérequis de PhantomJS, non ?

README à jour. Je vais envoyer un mail aux dev pour prévenir du changement de version. Je n'ai pas réussi à supprimer la dépendance globale.

jbuget · 2017-01-03T14:25:44Z

live/tests/integration/components/challenge-stay-test.js

+  });
+
+  it('renders', function() {
+    // Set any properties with this.set('myProperty', 'value');


code mort ?

coveralls · 2017-01-04T10:47:34Z

Coverage increased (+9.3%) to 92.833% when pulling 5f8f51b on 190-no-internet into 6420662 on dev.

jbuget · 2017-01-04T10:47:52Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2017-01-04T11:02:21Z

Changes Unknown when pulling 85fc3fa on 190-no-internet into ** on dev**.

jbuget · 2017-01-04T11:02:36Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2017-01-04T13:57:46Z

Changes Unknown when pulling a4d5c68 on 190-no-internet into ** on dev**.

jbuget · 2017-01-04T13:58:03Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2017-01-04T13:59:28Z

Changes Unknown when pulling a4d5c68 on 190-no-internet into ** on dev**.

jbuget · 2017-01-04T13:59:40Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2017-01-04T14:20:36Z

Changes Unknown when pulling 8554134 on 190-no-internet into ** on dev**.

jbuget · 2017-01-04T14:20:52Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

coveralls · 2017-01-06T12:43:33Z

Changes Unknown when pulling 7507fb9 on 190-no-internet into ** on dev**.

jbuget · 2017-01-06T12:43:52Z

I've deployed this PR to http://190-no-internet.pix.beta.gouv.fr. Please check it out

bdavidxyz added the [Tech] Review asked label Dec 22, 2016

jbuget suggested changes Jan 3, 2017

View reviewed changes

bdavidxyz added 16 commits January 4, 2017 11:57

transfert of lodash 3 to lodash 4, wip

e5a8149

drop ember-lodash, that was bound to old lodash 3

bab28d6

add comments

4ae9be2

remove relative paths

50040f7

severely improve utility function through lodash extension

915ac17

wip

59b6ccf

add test

8ee85a7

add field hasInternetAndTools in backend

75478f4

setup test ok

49502b0

clean code

c9c17b3

front ok

7fad51e

add condition to banner-warning display

c005638

corrected ESLint

c6db7cc

rewrite labeled checkboxes

5c800ad

rewrite labeled checkboxes , test ok

a6a770d

added tests

aa1357a

bdavidxyz added 5 commits January 4, 2017 11:57

cleaner

9b1fc3e

added comments

1b02a0a

correcting phantomjs version

6e26032

cleaner

6fcb1fe

improvments

85fc3fa

bdavidxyz force-pushed the 190-no-internet branch from 5f8f51b to 85fc3fa Compare January 4, 2017 10:57

bdavidxyz added 2 commits January 4, 2017 14:52

debugged

4bcda09

alignments

a4d5c68

debugged, 2

8554134

bdavidxyz added 3 commits January 6, 2017 11:34

backend corrected

7bff0d4

manual test ok

60ebdf8

lint correction

7507fb9

bdavidxyz added [Tech] Changes validated and removed [Tech] Review asked labels Jan 6, 2017

bdavidxyz merged commit 476cbc7 into dev Jan 6, 2017

bdavidxyz deleted the 190-no-internet branch January 6, 2017 14:49

mackwic added this to the no-milestone milestone Dec 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

US-190 avec ou sans internet #217

US-190 avec ou sans internet #217

bdavidxyz commented Dec 21, 2016

coveralls commented Dec 21, 2016

jbuget commented Dec 21, 2016

coveralls commented Dec 21, 2016

jbuget commented Dec 21, 2016

coveralls commented Dec 21, 2016

jbuget commented Dec 21, 2016

coveralls commented Dec 26, 2016

jbuget commented Dec 26, 2016

coveralls commented Dec 28, 2016

jbuget commented Dec 28, 2016

jbuget left a comment

jbuget Jan 3, 2017

jbuget Jan 3, 2017

jbuget Jan 3, 2017

jbuget Jan 3, 2017

jbuget Jan 3, 2017

bdavidxyz Jan 4, 2017

jbuget Jan 3, 2017

jbuget Jan 3, 2017

bdavidxyz Jan 4, 2017

jbuget Jan 3, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 6, 2017

jbuget commented Jan 6, 2017

US-190 avec ou sans internet #217

US-190 avec ou sans internet #217

Conversation

bdavidxyz commented Dec 21, 2016

coveralls commented Dec 21, 2016

jbuget commented Dec 21, 2016

coveralls commented Dec 21, 2016

jbuget commented Dec 21, 2016

coveralls commented Dec 21, 2016

jbuget commented Dec 21, 2016

coveralls commented Dec 26, 2016

jbuget commented Dec 26, 2016

coveralls commented Dec 28, 2016

jbuget commented Dec 28, 2016

jbuget 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

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 4, 2017

jbuget commented Jan 4, 2017

coveralls commented Jan 6, 2017

jbuget commented Jan 6, 2017