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

Fix eslint warnings #1254

Merged
merged 12 commits into from
Nov 18, 2022
Merged

Fix eslint warnings #1254

merged 12 commits into from
Nov 18, 2022

Conversation

lucapette
Copy link
Contributor

Built on top of #1251

Opening it as a draft because it's expected to fail until 1) we merge #1251 2) we figure out the best strategy to do this.

I'll be doing a couple of tests to confirm some hypothesis and then come back here and comment on 2)

@lucapette
Copy link
Contributor Author

In the last commit I did at first pass at prop types. I think I should be able to finish this in the morning tomorrow 💪

@lucapette
Copy link
Contributor Author

Done 🎉

I added prop types everywhere they were missing.

Now that's done, I'd wait for:

Once we're there, I will rebase this branch and add one "noisy" prettier commit (it will "fix" more than 70 files..).

It's the best way to solve all minor problems with eslint since we run prettier that way and it hasn't been run in a while I think.

I took some notes about the code I read while pushing through the prop types. All small things I can do in tiny pull requests (which is my fav way of contributing... I know it didn't show so far :D) once we're done with this.

Looking forward to next steps!

@tchiotludo
Copy link
Owner

release is done and merge of #1251 is also done !!
Go crazy man ! 😆

@lucapette lucapette marked this pull request as ready for review November 10, 2022 09:47
@lucapette
Copy link
Contributor Author

lucapette commented Nov 10, 2022

It's green now 🎉 🎉 🎉

While fixing the warnings, I learned a few things:

  • the eslint fix command was broken (I fixed it here but running it did not fix much)
  • the prettier configuration doesn't work well which means npx run prettier --write . right now fails. More about this later.
  • moving from node-sass to sass exposed some warnings in third-party libraries. I fixed the boostrap one by upgrading it. Font Awesome has the same problem but the fix is available only for version 6.

This pull request is still a little noisier than I'd like but to be fair I'm not sure I could break it down further. After all, it's a lot of small fixes that accumulated over time because the CI part wasn't that strict (before we upgrade react scripts).

I took a lot of notes about small code things. I'll be opening small pull-requests as I find the time (and hopefully also contributed a little to the java part. Would love to get some experience with micronaut).

About prettier, I can (and would love) to fix that myself but it only works if you are comfortable with another noise pull request (TIL we can ignore the commit which is very nice).

I can open a pull request as soon as we merge this one so it's also easier to see how that looks like. I like the "fix all things in one go" strategy because it makes upcoming pull-requests less noisy. But, of course, we'll do it whatever way works best for you!

@tchiotludo
Copy link
Owner

Amazing work !! Tested, it's seems that you don't break the app, even amazing!
I've added a small fix that must be due to bootstrap update.

I've merged that, thanks for amazing work here !

for the prettier things, go for it, no worries, and no need to remove from git blame, it's the history of the project, and sometimes we need to add a large noise!

Since we love large refactoring, maybe you can update react and others dependencies to last version 🤞 (I know I abuse but we are doing that large change without errors, love that ❤️ )

@tchiotludo tchiotludo merged commit 7f4f917 into tchiotludo:dev Nov 18, 2022
@lucapette
Copy link
Contributor Author

@tchiotludo really happy we merged this! I'll do the prettier thing as soon as I can.

It's funny that you mention the react upgrade I was afraid of bringing that up :D since it's again a "big change". But maybe not... I will look into it 💪

@tchiotludo
Copy link
Owner

To be honest, my main competence is definitely not on React, I'm more fluent with vuejs.
In react, if it work, I'm fine, If I need to go deeper on the react ecosystem, i'm completely lost.

But right now, I'm struggling with large update of vue (that is not fun) 😢

@lucapette
Copy link
Contributor Author

@tchiotludo I'm exactly the other way around so I'm happy to help. It feels good to give back to a project I have used for years!

12ushan added a commit to giffgaff/akhq that referenced this pull request Nov 22, 2022
* fix(topicdata): handle unsupported DescribeLogDirs  for MSK Serverless (tchiotludo#1113)

close tchiotludo#1112

* chore(deps): upgrade aws-msk-iam-auth to 1.1.4 to avoid vulnerability (tchiotludo#1114)

https://github.com/aws/aws-msk-iam-auth/releases/tag/v1.1.4 shows several fixes for CVEs

* chore(docs): add tui to usage list (tchiotludo#1118)

* chore(docs): update alt text for tui (tchiotludo#1119)

* chore(docs): add tui logo to public assets (tchiotludo#1120)

* feat(topicdata): adding serializer for protobuf schema registry (tchiotludo#1117)

* feat(topicdata): allow integer/long to be serialized as float/double when using AVRO schema. (tchiotludo#1123)

close tchiotludo#1122

* feat(ui): add pagination size of topic list (tchiotludo#1109)

relate tchiotludo#1051

* feat(acsl): add  Pattern Type to ACLs Panel Information (tchiotludo#1125)

close tchiotludo#1115

* fix(docker): run upgrade in order to reduce CVEs (tchiotludo#1134)

libssl1.1
dpkg
libldap-common

close tchiotludo#1132

* feat(docker): add healthcheck (tchiotludo#1136)


Co-authored-by: Ludovic DEHON <[email protected]>

* fix(ui): handle consumer group with slash (tchiotludo#1143)

close tchiotludo#1101

* Revert "chore(readme): add redpanda sponsors"

This reverts commit ab9a444.

* fix(helm): adding namespace to kubectl port-forward Helm Chart NOTES (tchiotludo#1171)

Co-authored-by: rogerio <[email protected]>

* fix(helm): correct port for port-forward is 8080 (tchiotludo#1178)

Fix port displayed at the end of the helm release.

It displays `{{ .Values.service.port }}` which is `80` per default, it should be `8080`.

* feat(docker): change upstream image from openjdk to eclipse-temurin (tchiotludo#1179)


Signed-off-by: Erik Godding Boye <[email protected]>

* fix(core): close consumer when returning null after calculating offset for newest sort (tchiotludo#1069)

fix a memory leak 

Co-authored-by: Neeraj.singh <[email protected]>

* fix(ui): load, display and store settings on settings screen if no settings have been stored yet (tchiotludo#1161)

Co-authored-by: David Müller <[email protected]>

* feat(docs): update vuepress to last versions

* chore(docs): add Fresha to whos using (tchiotludo#1111)

* chore(version): update to 0.22.0

* fix(ui): showing protobuf schema in versions tab crash (tchiotludo#1189)

Fixes tchiotludo#1188

* feat(docs): helm example with basic auth and aws msk (tchiotludo#1192)


Co-authored-by: Ludovic DEHON <[email protected]>

* feat(helm): add networkpolicy (tchiotludo#1193)

Signed-off-by: Quan TRAN <[email protected]>

* feat(helm): add configuration for readiness & livenessProbe

* chore(deps) add missing scala dependent modules (tchiotludo#1223)

* feat(docs): add a Kestra banner

* fix(topicdata): protection against tombstone message and headers

close tchiotludo#1210

* feat(webserver): add custom headers configuration (tchiotludo#1235)


Co-authored-by: Ludovic DEHON <[email protected]>

* chore(docs): fix typo (tchiotludo#1240)

* chore(cicd): update slack channel

* fix(ui): decimals are not parsed correctl (tchiotludo#1246)

replaced 'json-bigint' parser with 'lossless-json', this replacement allows for pretty-printing the json data, but still keeping the original values especially for floating numbers and bigints.

fix tchiotludo#1006

* core(deps): Updated micronaut to latest version (tchiotludo#1247)

and fixes to tests to have a successful build

close tchiotludo#1215

* fix(ui): Disabled edit access topic configs when role has reader mode (tchiotludo#1237)

close tchiotludo#1219

* fix(ui): encode groupId to allow '&' as part of the name of a consumer group (tchiotludo#1184)


relate to tchiotludo#1143

* feat(ui): faster topic-data search & sorting (tchiotludo#1209)


Co-authored-by: Max Bebök <[email protected]>

* feat(topicdata): added fix for incorrect datatype and missing fields in the json while producing to topic (tchiotludo#1233)

* feat(ui): Use `set INLINE_RUNTIME_CHUNK=false&& ` in react build. (tchiotludo#1238)

See also https://drag13.io/posts/react-inline-runtimer-chunk/index.html. This is to make _Content-Security-Policy_ work when `unsafe-inline` is not set.

Co-authored-by: Ludovic DEHON <[email protected]>

* fix(node):  fix nullpointer when trying to view cluster information in MSK Serverless (tchiotludo#1227)

close tchiotludo#1226

Co-authored-by: Ludovic DEHON <[email protected]>

* chore(deps): update all java deps

* feat(ui): optimze svgs with https://jakearchibald.github.io/svgomg/ (tchiotludo#1252)

In preparation for a React upgrade, see tchiotludo#1243

* feat(topicdata): support duplicate header keys (tchiotludo#1258)

close tchiotludo#1257 

Co-authored-by: rafanyan <[email protected]>

* chore(deps): update codeql actions to supported version (tchiotludo#1253)

See
https://github.blog/changelog/2022-04-27-code-scanning-deprecation-of-codeql-action-v1/
for context

* chore(version): update to 0.23.0

* feat(ui): update node to the latest lts version (tchiotludo#1251)

* Update node to the latest LTS version
* Replace node-sass with sass
* Also use the correct version of react-test-renderer.
* Update react scripts so we don't use insecure hashing functions
* Do not check eslint for production build

See tchiotludo#1243 for more information.

* fix(topicdata): fix null key and value (tchiotludo#1261)

Co-authored-by: alozano3 <[email protected]>

* feat(ui): fix eslint warnings (tchiotludo#1254)


Co-authored-by: Ludovic DEHON <[email protected]>

* fix(topci): create topics with configuration in one call (tchiotludo#1273)

close tchiotludo#1272

Signed-off-by: Erik Godding Boye <[email protected]>
Signed-off-by: Quan TRAN <[email protected]>
Co-authored-by: Mitsuaki Ito <[email protected]>
Co-authored-by: tooptoop4 <[email protected]>
Co-authored-by: Steven Masala <[email protected]>
Co-authored-by: Steven Masala <[email protected]>
Co-authored-by: Andrei Strelnikov <[email protected]>
Co-authored-by: Marcello <[email protected]>
Co-authored-by: 10xtechie <[email protected]>
Co-authored-by: ThomasSanson <[email protected]>
Co-authored-by: Ludovic DEHON <[email protected]>
Co-authored-by: Rodrigo Rodriguez Ramos <[email protected]>
Co-authored-by: Rogério Fonseca <[email protected]>
Co-authored-by: rogerio <[email protected]>
Co-authored-by: Thomas <[email protected]>
Co-authored-by: Erik Godding Boye <[email protected]>
Co-authored-by: neeraj-singh47 <[email protected]>
Co-authored-by: Neeraj.singh <[email protected]>
Co-authored-by: sam0r040 <[email protected]>
Co-authored-by: David Müller <[email protected]>
Co-authored-by: Piotr Rybarczyk <[email protected]>
Co-authored-by: Ali Akhtari <[email protected]>
Co-authored-by: Alex Vaque <[email protected]>
Co-authored-by: Quan TRAN <[email protected]>
Co-authored-by: Adi Wehrli <[email protected]>
Co-authored-by: lucapette <[email protected]>
Co-authored-by: tristanessquare <[email protected]>
Co-authored-by: meeraj257 <[email protected]>
Co-authored-by: Gnana_Jeyam <[email protected]>
Co-authored-by: owidder <[email protected]>
Co-authored-by: Stuff is on GitLab <[email protected]>
Co-authored-by: Max Bebök <[email protected]>
Co-authored-by: Raphael <[email protected]>
Co-authored-by: rafanyan <[email protected]>
Co-authored-by: Albert <[email protected]>
Co-authored-by: alozano3 <[email protected]>
Co-authored-by: Emmanuel <[email protected]>
tchiotludo added a commit that referenced this pull request Apr 4, 2023
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.

2 participants