You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@annotator.apache.org by tilgovi <gi...@git.apache.org> on 2017/05/09 05:53:34 UTC

[GitHub] incubator-annotator pull request #3: Prettier

GitHub user tilgovi opened a pull request:

    https://github.com/apache/incubator-annotator/pull/3

    Prettier

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/incubator-annotator prettier

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-annotator/pull/3.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3
    
----
commit 9f0175871ed995d2f77a5fceaf12217238b4d6fd
Author: Randall Leeds <ra...@apache.org>
Date:   2017-05-09T04:56:53Z

    Add prettier

commit ac6c1bb294ad393f97fb64f28477e5200970c5f3
Author: Randall Leeds <ra...@apache.org>
Date:   2017-05-09T04:58:16Z

    Upgrade yarn lockfile

commit 504b1abac1ae440de16c2e868c15c650397f2227
Author: Randall Leeds <ra...@apache.org>
Date:   2017-05-09T04:58:35Z

    Fix prettier lint errors

commit e7282de68446d73370b7c8eb34311f920da2ee2e
Author: Randall Leeds <ra...@apache.org>
Date:   2017-05-09T05:17:54Z

    Use more recommended eslint rules

commit 0a209fea2a8420bd4156460cdb5d54a4126c9cd8
Author: Randall Leeds <ra...@apache.org>
Date:   2017-05-09T05:39:38Z

    Add husky and lint-staged

commit 8ff52936344bc6e38b8e13645b6c760ed6be4561
Author: Randall Leeds <ra...@apache.org>
Date:   2017-05-09T05:39:45Z

    Upgrade yarn lockfile

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator pull request #3: Prettier

Posted by tilgovi <gi...@git.apache.org>.
Github user tilgovi commented on a diff in the pull request:

    https://github.com/apache/incubator-annotator/pull/3#discussion_r115547760
  
    --- Diff: scripts/.eslintrc.json ---
    @@ -1,7 +1,13 @@
     {
    +  "env": {
    +    "node": true
    --- End diff --
    
    This is the local .eslintrc.json for the scripts folder, container build scripts that run only in node.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator pull request #3: Prettier

Posted by BigBlueHat <gi...@git.apache.org>.
Github user BigBlueHat commented on a diff in the pull request:

    https://github.com/apache/incubator-annotator/pull/3#discussion_r115589406
  
    --- Diff: scripts/serve.js ---
    @@ -39,19 +39,16 @@ const packages = fs.readdirSync(packagesPath);
     
     const app = express();
     
    -packages.forEach((name) => {
    +packages.forEach(name => {
       const root = path.join(packagesPath, name);
       const entry = path.join(root, 'index.js');
       const dest = path.join(root, `${name}.bundle.js`);
    -  const external = (id) => /^@(annotator|hot)/.test(id);
    +  const external = id => /^@(annotator|hot)/.test(id);
    --- End diff --
    
    Works for me. Thanks for the explanation @robertknight 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator pull request #3: Prettier

Posted by robertknight <gi...@git.apache.org>.
Github user robertknight commented on a diff in the pull request:

    https://github.com/apache/incubator-annotator/pull/3#discussion_r115536537
  
    --- Diff: scripts/serve.js ---
    @@ -39,19 +39,16 @@ const packages = fs.readdirSync(packagesPath);
     
     const app = express();
     
    -packages.forEach((name) => {
    +packages.forEach(name => {
       const root = path.join(packagesPath, name);
       const entry = path.join(root, 'index.js');
       const dest = path.join(root, `${name}.bundle.js`);
    -  const external = (id) => /^@(annotator|hot)/.test(id);
    +  const external = id => /^@(annotator|hot)/.test(id);
    --- End diff --
    
    This is how prettier formats arrow functions with a single argument. There is [no option](https://github.com/prettier/prettier#options) to change this.
    
    By design, prettier provides only a handful of options to choose major style options such as single vs. double quotes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator pull request #3: Prettier

Posted by BigBlueHat <gi...@git.apache.org>.
Github user BigBlueHat commented on a diff in the pull request:

    https://github.com/apache/incubator-annotator/pull/3#discussion_r115521367
  
    --- Diff: scripts/.eslintrc.json ---
    @@ -1,7 +1,13 @@
     {
    +  "env": {
    +    "node": true
    --- End diff --
    
    We should probably use `shared-node-browser` for `env`:
    http://eslint.org/docs/user-guide/configuring#specifying-environments
    
    We can discuss the audience of each package on the mailing list.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator issue #3: Prettier

Posted by tilgovi <gi...@git.apache.org>.
Github user tilgovi commented on the issue:

    https://github.com/apache/incubator-annotator/pull/3
  
    Merged with the change to use "off" instead of "0".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator pull request #3: Prettier

Posted by BigBlueHat <gi...@git.apache.org>.
Github user BigBlueHat commented on a diff in the pull request:

    https://github.com/apache/incubator-annotator/pull/3#discussion_r115522283
  
    --- Diff: scripts/serve.js ---
    @@ -39,19 +39,16 @@ const packages = fs.readdirSync(packagesPath);
     
     const app = express();
     
    -packages.forEach((name) => {
    +packages.forEach(name => {
       const root = path.join(packagesPath, name);
       const entry = path.join(root, 'index.js');
       const dest = path.join(root, `${name}.bundle.js`);
    -  const external = (id) => /^@(annotator|hot)/.test(id);
    +  const external = id => /^@(annotator|hot)/.test(id);
    --- End diff --
    
    Is the removal of parens from them `google` presets? I actually rather like having them around as I find hunting for the extra `>` in a line tedious.
    
    But again...mailing list? :wink:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator pull request #3: Prettier

Posted by tilgovi <gi...@git.apache.org>.
Github user tilgovi closed the pull request at:

    https://github.com/apache/incubator-annotator/pull/3


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-annotator pull request #3: Prettier

Posted by BigBlueHat <gi...@git.apache.org>.
Github user BigBlueHat commented on a diff in the pull request:

    https://github.com/apache/incubator-annotator/pull/3#discussion_r115522021
  
    --- Diff: scripts/.eslintrc.json ---
    @@ -1,7 +1,13 @@
     {
    +  "env": {
    +    "node": true
    +  },
       "globals": {
         "__dirname": false,
         "__filname": false,
         "__moduleName": true
    +  },
    +  "rules": {
    +    "no-console": 0
    --- End diff --
    
    Could we use the eslint "magic string" value of `"off"` here? It's clearer once we start mixing in `"warn"` and `"error"`: http://eslint.org/docs/user-guide/configuring#configuring-rules


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---