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

Make the dev build work on apple m1 #1243

Closed
wants to merge 1 commit into from

Conversation

lucapette
Copy link
Contributor

First of all, thank you very much for this amazing project. I'm a long time user and big fan of the project!

Yesterday I started reading the source code to understand how you approach some problems (I'm working on a side-project with overlapping challenges. For example, I'm interested in how you "count" records in a topic).

At some point reading didn't do it anymore and I tried to run the project locally which lead to this pull-request.

If you look at the diff, it feels there's a variety of unrelated changes bundled up in one pull-request. But they're all "build related", let me explain.

In practice, what happened is that I run into a few issues one after the other before I could successfully run gradlew build (which still "fails" because of eslint errors. More on this later) and docker-compose -f docker-compose-dev.yml up (as suggested in the dev.md doc).

So here's the timeline of the problems I run into (with their relative fixes):

While doing this, I also updated the docs/docs/dev.md a little (using markdownlint-cli rules as a loose reference).

With these changes, I can run docker-compose -f docker-compose-dev.yml up but still have problems with gradlew build.

The problem is that there's a number of eslint errors to fix.

I can fix the eslint errors myself. I enjoy these tasks because they make me more familiar with a code base while allowing me to think "big picture" (it's some sort of "programming meditation" for me).

I'm reaching out now with a draft pull-request because it doesn't makes sense to me to invest more time into this unless:

  1. You're OK with the direction of the changes I already made.
  2. This pull-request getting a bigger diff.
  3. We agree on how to fix the eslint errors (Personally, I'm not sure I'd do the prop-types ones... but that's my bias toward "solving" that by using TypeScript which is a whole different conversation. Happy to have that one as well :))

If we work on this pr and merge it (I really hope so, I've been meaning to give back to this project of a while now), here's what I'd after it:

  • I add a github workflow that ensures no eslint problems get merged anymore.
  • I would do the same exact things with docs (by using markdownlint, I use it in all my projects and it's nice to have a "format on save" option in vs code or intellij for this too)

Looking forward to hearing your thoughts on this!

@tchiotludo
Copy link
Owner

Thanks for the very detailed description!
First of all, most of the comment is really relevant!

Let's dig inside the issues but most of them will required separated PR to avoid the massive one and a proper git history! (and some change are easy to be merged)

  • Update the node version: Go for it, no problem (PR 1)
  • replace node-sass to sass: same no problem, I was thinking there is more tunning on react side (PR2)
  • The image: same no problems (PR3)
  • Update the docs: no problems also (PR4)

The last one! the update of React!

I can fix the eslint errors myself. I enjoy these tasks

Oh my god, you will rescue me for a long long pain 🙏 In fact, I don't have the time to update the react version because of this lint error, ... and to be honest, it begin to be a real mess right now. If you have the time and the desire, I will be really thankful for a complete life, dependabot is crying on me for too many times!

Tell me what is your thinking and thanks!

@lucapette
Copy link
Contributor Author

yes this makes a lot of sense to me. I opened a draft pr with a long description so I could provide context about all the changes (they do seem disconnected otherwise).

I will create a bunch of little pull-requests in preparation of the React update. Once we get there, we can discuss how to best approach the eslint errors (some have) obvious fixes, others don't.

I'm closing this pr as we don't need it anymore :)

Looking forward to getting all of this done!

@lucapette lucapette closed this Nov 7, 2022
lucapette added a commit to lucapette/akhq that referenced this pull request Nov 7, 2022
lucapette added a commit to lucapette/akhq that referenced this pull request Nov 7, 2022
Also use the correct version of react-test-renderer.

See tchiotludo#1243 for more information.
tchiotludo pushed a commit that referenced this pull request Nov 7, 2022
lucapette added a commit to lucapette/akhq that referenced this pull request Nov 7, 2022
Also use the correct version of react-test-renderer.

See tchiotludo#1243 for more information.
tchiotludo pushed a commit that referenced this pull request Nov 9, 2022
* 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 #1243 for more information.
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 pushed a commit that referenced this pull request Apr 4, 2023
* 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 #1243 for more information.
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