-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
Hey @lucashanke, I had to re-configure it because Azure messed it up a bit (I renamed the repo 😅 so the builds were mixed up). Also, now our build is public and you is part of the team. Can you try accessing it again please? https://dev.azure.com/apgomes88/Pardal/_build/results?buildId=48 |
It's fine now, @anapaulagomes! Thanks ;) The step is failling due to lint issues. We need to fix them. Can we wait to that after #19 is merged? |
Sure thing, @lucashanke. I'll take a look tonight. |
9e8110f
to
0014385
Compare
It took me a while to understand that the linter is working fine. 😂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it looks great, @anapaulagomes! Thanks for fixing all the linting issues. I just left a comment regarding trailing commas, which is a common practice and I find them quite clean. https://github.com/airbnb/javascript#commas--dangling
screen_name: ['user', 'screen_name'], // nested keys | ||
message: ['full_text'], | ||
when: ['created_at'], | ||
from: ['source'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linter is complaining about trailing commas? I think they're part of Airbnb style guides. And I quite like them as well. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, this was fixed by npm run lint -- --fix
. I liked it too. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we change something on our .eslintrc.json
? It should be applying airbnb-style, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in theory. I've jest removed the prettier plugin and eslint now is complaining about missing trailing comma.
Apparently prettier is overriding some rules. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Figured it out! It is the order of the 'extends' option in eslint config file that dictates the priority of the rules. Just pushed the change. Can you check if it's complaining about trailing commas for you too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just saw that CI was failing due to the missing trailing commas. ;)
Just fixed the issues and pushed the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Thanks for your help. 💃
Since the mystery of trailing commas was solved, I'm gonna merge this. 🎉 |
Resolves #4.