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

Remove Restyle in CML or adjust it to follow airbnb style #596

Closed
DavidGOrtega opened this issue Jun 16, 2021 · 6 comments
Closed

Remove Restyle in CML or adjust it to follow airbnb style #596

DavidGOrtega opened this issue Jun 16, 2021 · 6 comments
Labels
technical-debt Refactoring, linting & tidying

Comments

@DavidGOrtega
Copy link
Contributor

DavidGOrtega commented Jun 16, 2021

Having some clear difficulties in having consistency in style, its better to just let the linter speak and refuse the PR if the style does not match the chosen style.

We have to:

  • Remove Restyle or make it use our eslint config (I remember that to not be possible)

This will make contributions harder but at least the base source code will follow the same convention.

@DavidGOrtega DavidGOrtega added the technical-debt Refactoring, linting & tidying label Jun 16, 2021
@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Jun 16, 2021

Existential doubt from #581 (comment), #608 (comment) and #648 (comment) about what is the best approach for accessing optional nested values:

Perlification1

(item) => {
  const { property: { data = '' } = {} } = item;
  return data.endsWith('text');
}

Abracadabra

(item) =>
  item &&
  item.property &&
  item.property.data &&
  item.property.data.endsWith('text');

Chaining2

(item) => item?.property?.data?.endsWith('text')

1 Works for empty objects, but not for nullish values
2 Only valid on Node.js 14+ as per #597

@0x2b3bfa0
Copy link
Member

Note: as per #581 (comment), we probably should broaden a bit the scope of the issue title.

0x2b3bfa0 added a commit that referenced this issue Jun 22, 2021
* Fix minor typo in cml-send-comment.js
* Add GitHub support for cml-send-comment --update
* Add BitBucket support for cml-send comment --update
* Increase end to end test timeouts
* Emit a warning when Pull Request Commit Links is not installed
* Alias console.log to print after monkey-patching
* Rename fullEndpoint to url
* Use more destructuring on bitbucket_cloud.js
* Use destructuring on github.js
* Add GitLab error for comment updates
* Use destructuring as a workaround to optional chaining (#596)
* Require the Pull Request Commit Links API to work
* BB comment spacing
@0x2b3bfa0
Copy link
Member

Another style topic: #583 (comment)

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Jun 25, 2021

Speaking about what linters can't detect: how deffensive should we be when writing code that interacts with complex systems we can't control?

@0x2b3bfa0
Copy link
Member

Already using optional chaining 🎉

@0x2b3bfa0
Copy link
Member

Tracking #1026 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical-debt Refactoring, linting & tidying
Projects
None yet
Development

No branches or pull requests

3 participants