Skip to content

issue #1376 More consistent formatting#1431

Merged
tomholub merged 62 commits intomasterfrom
ip-1376-formatting
Apr 9, 2022
Merged

issue #1376 More consistent formatting#1431
tomholub merged 62 commits intomasterfrom
ip-1376-formatting

Conversation

@IvanPizhenko
Copy link
Contributor

@IvanPizhenko IvanPizhenko commented Mar 21, 2022 •
edited by tomholub
Loading

This PR adds some settings for formatting

close #1376

Tests (delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@IvanPizhenko IvanPizhenko changed the title Ip 1376 formatting issue #1376 More consistent formatting Mar 21, 2022
@IvanPizhenko IvanPizhenko mentioned this pull request Mar 21, 2022
Closed
@tomholub
Copy link
Collaborator

tomholub commented Mar 22, 2022

That looks ok, now you want to make sure that the CI tests for it, like on browser extension:

      jobs:

        - name: code quality
          commands:
            - npm run-script test_tslint
            - npm run-script test_eslint
            - npm run-script test_stylelint

@tomholub
Copy link
Collaborator

tomholub commented Mar 22, 2022

You'll also want to copy large part of the workspace file - recommeded extensions and workpace style settings:

https://github.com/FlowCrypt/flowcrypt-browser/blob/master/flowcrypt-browser.code-workspace

@IvanPizhenko
Copy link
Contributor Author

IvanPizhenko commented Mar 23, 2022

There are lots of warnings from running eslint and tslint, so I'm going to fix that.

Ivan Pizhenko added 2 commits March 24, 2022 21:47
@tomholub
Copy link
Collaborator

tomholub commented Mar 24, 2022

To self-correct, - npm run-script test_stylelint is not relevant for this repo

Ivan Pizhenko added 2 commits April 5, 2022 23:59
@lgtm-com
Copy link

lgtm-com bot commented Apr 5, 2022

This pull request introduces 1 alert when merging dee0035 into 628006b - view on LGTM.com

new alerts:

  • 1 for Template syntax in string literal

tomholub
tomholub reviewed Apr 6, 2022
View reviewed changes
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

120 is way too short. But I'll live with it. See comments

@lgtm-com
Copy link

lgtm-com bot commented Apr 6, 2022

This pull request introduces 1 alert when merging c7de4e9 into e31a595 - view on LGTM.com

new alerts:

  • 1 for Template syntax in string literal

@lgtm-com
Copy link

lgtm-com bot commented Apr 6, 2022

This pull request introduces 1 alert when merging d86591d into e31a595 - view on LGTM.com

new alerts:

  • 1 for Template syntax in string literal

@lgtm-com
Copy link

lgtm-com bot commented Apr 6, 2022

This pull request introduces 1 alert when merging 3089e81 into e31a595 - view on LGTM.com

new alerts:

  • 1 for Template syntax in string literal

Ivan Pizhenko added 2 commits April 6, 2022 23:36
@lgtm-com
Copy link

lgtm-com bot commented Apr 6, 2022

This pull request introduces 1 alert when merging 61279a6 into e31a595 - view on LGTM.com

new alerts:

  • 1 for Template syntax in string literal

@lgtm-com
Copy link

lgtm-com bot commented Apr 6, 2022

This pull request introduces 1 alert when merging f83f39c into e31a595 - view on LGTM.com

new alerts:

  • 1 for Template syntax in string literal

@lgtm-com
Copy link

lgtm-com bot commented Apr 6, 2022

This pull request introduces 1 alert when merging a614bc1 into e31a595 - view on LGTM.com

new alerts:

  • 1 for Template syntax in string literal

Ivan Pizhenko added 2 commits April 7, 2022 01:52
@IvanPizhenko IvanPizhenko marked this pull request as ready for review April 8, 2022 20:26
@IvanPizhenko
Copy link
Contributor Author

IvanPizhenko commented Apr 8, 2022

@tomholub Swift tests also fail here

@tomholub
Copy link
Collaborator

tomholub commented Apr 9, 2022

They should stabilize once #1480 merges today

tomholub
tomholub approved these changes Apr 9, 2022
View reviewed changes
Core/source/test.ts
variant: 'curve25519', passphrase: 'riruekfhydekdmdbsyd',
userIds: [{ email: 'a@b.com', name: 'Him' }]
}));
// tslint:disable:no-unsafe-any
Copy link
Collaborator

@tomholub tomholub Apr 9, 2022

Choose a reason for hiding this comment

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

no-unsafe-any should not apply to tests. Can you exclude all tests from this rule?

Copy link
Collaborator

@tomholub tomholub Apr 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Alternatively, can put one no-unsafe-any exception on top of every test file that needs it

Copy link
Contributor Author

@IvanPizhenko IvanPizhenko Apr 9, 2022

Choose a reason for hiding this comment

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

Not sure how to disable specific rule for the specific file. Yes, I think it's easier to disable rule right in the file.

@tomholub tomholub enabled auto-merge (squash) April 9, 2022 07:50
@tomholub tomholub merged commit 2cfa95b into master Apr 9, 2022
@tomholub tomholub deleted the ip-1376-formatting branch April 9, 2022 19:02
@IvanPizhenko IvanPizhenko mentioned this pull request Apr 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomholub tomholub tomholub approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consistent code formatting

2 participants

@IvanPizhenko @tomholub