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

Add settings and info for developing UI in IntelliJ #1496

Merged
merged 8 commits into from
Jul 15, 2022

Conversation

aaronweissler
Copy link
Member

@aaronweissler aaronweissler commented Jul 11, 2022

This change is Reviewable

@aaronweissler aaronweissler force-pushed the javascript-codestyle branch from 225a1dc to 269e205 Compare July 11, 2022 15:08
PDuwe
PDuwe previously approved these changes Jul 11, 2022
Copy link

@PDuwe PDuwe left a comment

Choose a reason for hiding this comment

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

Some tests seem to be failing.

Also, you could still get rid of the warnings. Looks good apart from the tests however, so you decide :)

@aaronweissler
Copy link
Member Author

Some tests seem to be failing.

Those agent tests have been failing seemingly randomly for a small while now. We still will investigate that further, but it's definitely not related to this PR, since there are no changes to Java code or directly related things in here at all (which also was my motivation for adding additional ignore paths for the workflow in #1497 that you also reviewed ^^)

Also, you could still get rid of the warnings. Looks good apart from the tests however, so you decide :)

If you mean the warnings seen in Files Changed, they are also not related to this PR's changes, I'm not sure why GitHub is showing them for those two files.

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aaronweissler and @PDuwe)


CONTRIBUTING.md line 17 at r2 (raw file):

### Frontend Developing
If developing JavaScript in IntelliJ, check that Settings.Languages & Frameworks.JavaScript.Code Quality Tools.ESLint is activated for correct linting.

this feature is only available in IntelliJ Ultimate, right?

If so, please add Ultimate


components/inspectit-ocelot-configurationserver-ui/.eslintrc line 45 at r1 (raw file):

        "import/no-useless-path-segments": "error",
        "react/prop-types": "warn",
        "prettier/prettier": ["error", { "endOfLine": "auto" }]

this fixes the delete 'CR' [prettier/prettier] error message, right? (https://stackoverflow.com/questions/53516594/why-do-i-keep-getting-delete-cr-prettier-prettier)


components/inspectit-ocelot-configurationserver-ui/README.md line 33 at r2 (raw file):

And the following to fix them automatically.
```bash
yarn format:write

what does yarn format:write do as compared to yarn lint --fix?

Is it recommended to run eslint --fix on save?

@heiko-holz heiko-holz self-assigned this Jul 12, 2022
@heiko-holz
Copy link
Contributor

components/inspectit-ocelot-configurationserver-ui/README.md line 33 at r2 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

what does yarn format:write do as compared to yarn lint --fix?

Is it recommended to run eslint --fix on save?

(addendum: with current version of IntelliJ, we can use Tools.Actions on Save and check Run eslint --fix:
image.png

Copy link
Member Author

@aaronweissler aaronweissler left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @heiko-holz and @PDuwe)


CONTRIBUTING.md line 17 at r2 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

this feature is only available in IntelliJ Ultimate, right?

If so, please add Ultimate

Yes, you're right. Done.


components/inspectit-ocelot-configurationserver-ui/.eslintrc line 45 at r1 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

this fixes the delete 'CR' [prettier/prettier] error message, right? (https://stackoverflow.com/questions/53516594/why-do-i-keep-getting-delete-cr-prettier-prettier)

Yes, exactly, and, as far as I understand, git takes care of keeping the correct line endings in the repository, so it's okay to accept the Windows line endings locally.


components/inspectit-ocelot-configurationserver-ui/README.md line 33 at r2 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

(addendum: with current version of IntelliJ, we can use Tools.Actions on Save and check Run eslint --fix:
image.png

yarn format:write runs prettier --write fixing issues, while yarn lint --fix or yarn lint:fix run eslint --fix. So in this case it should be the lint command instead. If I understand correctly, the lint command also considers errors considered by prettier anyways.
I also like just using the auto apply, so you get your errors fixed while developing without having to think of running it yourself, and don't have to fix all of them at once in the end if you forget.

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r3.
Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @heiko-holz and @PDuwe)


components/inspectit-ocelot-configurationserver-ui/README.md line 33 at r2 (raw file):

Previously, aaronweissler wrote…

yarn format:write runs prettier --write fixing issues, while yarn lint --fix or yarn lint:fix run eslint --fix. So in this case it should be the lint command instead. If I understand correctly, the lint command also considers errors considered by prettier anyways.
I also like just using the auto apply, so you get your errors fixed while developing without having to think of running it yourself, and don't have to fix all of them at once in the end if you forget.

Thanks for the explanation!

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @PDuwe)

@heiko-holz heiko-holz merged commit c0a2e1f into inspectIT:master Jul 15, 2022
@aaronweissler aaronweissler deleted the javascript-codestyle branch July 18, 2022 08:35
aaronweissler added a commit to salim-1997/inspectit-ocelot that referenced this pull request Jul 25, 2022
* Activate formatter tags by default

* Add indentation settings for JS

* Add to CONTRIBUTING regarding JavaScript linting

* Fix eol linting of .js files on Windows

* Add yarn lint and format:write to README

* Add section in CONTRIBUTING.md

* Add to CONTRIBUTING based on comments

* Fix linting command in README

Co-authored-by: Heiko Holz <[email protected]>
salim-1997 pushed a commit to salim-1997/inspectit-ocelot that referenced this pull request Aug 15, 2022
* Activate formatter tags by default

* Add indentation settings for JS

* Add to CONTRIBUTING regarding JavaScript linting

* Fix eol linting of .js files on Windows

* Add yarn lint and format:write to README

* Add section in CONTRIBUTING.md

* Add to CONTRIBUTING based on comments

* Fix linting command in README

Co-authored-by: Heiko Holz <[email protected]>
salim-1997 pushed a commit to salim-1997/inspectit-ocelot that referenced this pull request Sep 1, 2022
* Activate formatter tags by default

* Add indentation settings for JS

* Add to CONTRIBUTING regarding JavaScript linting

* Fix eol linting of .js files on Windows

* Add yarn lint and format:write to README

* Add section in CONTRIBUTING.md

* Add to CONTRIBUTING based on comments

* Fix linting command in README

Co-authored-by: Heiko Holz <[email protected]>
aaronweissler added a commit that referenced this pull request Sep 5, 2022
* Activate formatter tags by default

* Add indentation settings for JS

* Add to CONTRIBUTING regarding JavaScript linting

* Fix eol linting of .js files on Windows

* Add yarn lint and format:write to README

* Add section in CONTRIBUTING.md

* Add to CONTRIBUTING based on comments

* Fix linting command in README

Co-authored-by: Heiko Holz <[email protected]>
aaronweissler added a commit that referenced this pull request Sep 5, 2022
* Activate formatter tags by default

* Add indentation settings for JS

* Add to CONTRIBUTING regarding JavaScript linting

* Fix eol linting of .js files on Windows

* Add yarn lint and format:write to README

* Add section in CONTRIBUTING.md

* Add to CONTRIBUTING based on comments

* Fix linting command in README

Co-authored-by: Heiko Holz <[email protected]>
aaronweissler pushed a commit that referenced this pull request Sep 5, 2022
… of Config Server UI dependencies (#1510)

* update patches

* update minor versions

* update eslint dependencies

* update @babel/core dependency

* re-update eslint

* update primeinons

* update primeflex

* update react-timeago

* update react-syntax-highligter

* update react-redux

* update jwt-decode

* Merge branch 'master' of https://github.com/inspectIT/inspectit-ocelot into dependencies-update

* Closes #1507 : Update minor versions, patches and some major
versions(#1507)

Highlight enums with String values correctly (#1488)

* Treat enums with values-field differently

* Fix default config exporters.yml

* Fix order of possible values in test

* Use toString instead of getName

* Fix JSON parsing of Maps in test

Using toString, empty values could not be parsed correctly by gson

update patches

update minor versions

update eslint dependencies

update @babel/core dependency

re-update eslint

update primeinons

update primeflex

update react-timeago

update react-syntax-highligter

update react-redux

* update jwt-decode

* Add settings and info for developing UI in IntelliJ (#1496)

* Activate formatter tags by default

* Add indentation settings for JS

* Add to CONTRIBUTING regarding JavaScript linting

* Fix eol linting of .js files on Windows

* Add yarn lint and format:write to README

* Add section in CONTRIBUTING.md

* Add to CONTRIBUTING based on comments

* Fix linting command in README

Co-authored-by: Heiko Holz <[email protected]>

* [skip ci] Publish documentation v2.1.0

* Fix release notes links (#1503)

* Closes #1507 - Update minor versions, patches and some major versions

* Closes #1507 - Update minor versions, patches and some major versions

* Delete package-lock.json

Only one lock-file should be in project, which is yarn.lock in our case.

* Small fix to next.config

* Fix license string

* Change parser to babel/eslint-parser

* Add peer dependencies

* Merge branch 'master' into dependencies-update

* Fix linting errors

* Move babel eslint to devdependencies, remove old version

* Add helper-string-parser to devdependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants