fedora-hubs

#207 Feed widget to external SSE and JSX style rules
Merged by atelic. Opened by atelic.
atelic/fedora-hubs fix/jsx-style  into  develop

Download 207.patch
  • In order to comply with JavaScript style standards, change 4 spaces to 2.
  • Adds eslint rules using Airbnb style guide
  • Moves Feed widget to use external SSE

is the rest of the html/js files 4 spaces or 2 spaces?
Edited by skrzepto

rebased

Yes, the consistent style for hubs and other fedora applications is 2 spaces for html/js

Yes, 2 spaces is what we maintain. So :thumbsup: for me

:thumbsup: for me as well

Cool, can you confirm #206 fixes the issue you were having earlier with the feed?

3 new commits added

  • Change feed widget to card format
  • Add class properties req and new data to top
  • Move Feed to external SSE

2 new commits added

  • Make feed limit configurable
  • JSX style rules from Airbnb style guide

Should this come from the configuration ?

Yes, it would be nice to have a config variable in the template environment for this. I also didn't want to commit skrzepto's IP.

we can user 'localhost' as a placeholder for now if that helps?

We can just access the flask config via {{ config }} in the templates

rebased

rebased

8 new commits added

  • Use application config to set the SSE url
  • Fix human readable time, clean up JS/JSX
  • Make feed limit configurable
  • JSX style rules from Airbnb style guide
  • Change feed widget to card format
  • Add class properties req and new data to top
  • Move Feed to external SSE
  • Move 4 spaces to 2 in JSX files

@pingou @sayanchowdhury This is ready for another round of review.

Just to summarize the changes here:

  • The majority of the changed files are just code indentation for the JSX files to use a consistent 2 spaces
  • Remove of a lot of the temporary things implemented before regarding the Feed widget (e.g. the api route)
  • Move to getting streamed messages from an external server. This value is changed in default_config.py and you will need to change it in order to test.
  • Add ability to configure limit for messages in Feed widget
  • Pin minimum requirement for flask-oidc

This line is too long I can't read it in the PR page w/o scrolling on the side

+-c config ?

Ah, right. I added this before the -c arg. Changing

rebased

8 new commits added

  • Use application config to set the SSE url
  • Fix human readable time, clean up JS/JSX
  • Make feed limit configurable
  • JSX style rules from Airbnb style guide
  • Change feed widget to card format
  • Add class properties req and new data to top
  • Move Feed to external SSE
  • Move 4 spaces to 2 in JSX files

Bringing up review on this again since this is now a blocker for #158

is this fmn_context still used?

FTR https://pagure.io/fork/atelic/fedora-hubs/blob/17896a1d6a2b7e814f0f779292b9e1ad7fad3310/f/hubs/static/client/app/components/Constituent.jsx and https://pagure.io/fork/atelic/fedora-hubs/blob/80ab303da3320d862cc8511766e2208a6e3b4c90/f/hubs/static/client/app/components/ExpandCollapse.jsx seems to be using 4 spaces indentation

Edit: same for: https://pagure.io/fork/atelic/fedora-hubs/blob/f034a87a7bda158599608f5b07e1cedbd419d728/f/hubs/static/client/app/components/Messages.jsx
Edited by pingou

No author?

Couple of comments but nothing else stands out for me, but I am definitely not the best person to review w/ my lack of understanding of the JS code :(

Maybe @sayanchowdhury could do another pass at it?

Hm, I hadn't thought about this before. After testing it a bit this morning, it looks like it isn't needed. Removed

rebased

1 new commit added

  • Fix tests fmn_context change + fedmsg api removal

1 new commit added

  • Explain seemingly unused import in index.jsx

Looked through the code :thumbsup: for me.

@atelic should we merge this now?

:ship:

Pull-Request has been merged by atelic
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.