-
Notifications
You must be signed in to change notification settings - Fork 18
Fix-js-linting-errors #90
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
base: master
Are you sure you want to change the base?
Conversation
@@ -18,17 +19,6 @@ import { Divider, Button, Grid, Header } from "semantic-ui-react"; | |||
import { AdminUIRoutes } from "@js/invenio_administration"; | |||
|
|||
export class JobRunsHeader extends Component { | |||
constructor(props) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: why has this been removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constructor was removed because title, description, config
and loading are now passed as props instead of managed in local state. The component no longer mutates them internally, so keeping a state copy was redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but where are they passed as props, i couldn't find that in the diff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The props are passed via the data object see: here (containing title and description) and the direct loading prop.
So the parent component now manages these values instead of local state.
<JobRunsHeader
data={{
title: "My Job Title", // Fills data.title
description: "Job description", // Fills data.description
links: { /*...*/ }
}}
loading={false} // Direct loading prop
/* other props... */
/>
<Message.Item>The job has not produced any logs yet.</Message.Item> | ||
<Message.Item>Logs were deleted due to the retention policy.</Message.Item> | ||
<Message.Item> | ||
The job has not produced any logs yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: should this be marked as translatable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed these in another PR: #93
- Added JS job to the CI workflow for testing JavaScript files. - add node_modules to gitignore
- Removed unused constructor and state initialization. - Updated propTypes and defaultProps for clarity. - Added missing import for Actions.
the following is being fixed: * error 'loading' is missing in props validation * error 'formData' is missing in props validation * error 'error' is missing in props validation * error Identifier 'actions_errors' is not in camel case camelcase * error Identifier 'actions_errors' is not in camel case camelcase * error 'error.content' is missing in props validation
* Fix memory leak by properly canceling requests on unmount * Initialize derived state in constructor * Add log deduplication using timestamp-level-message keys * Use separate cancel tokens for different requests * Replace array index keys with stable composite keys * fix linter errors and warnings
* fix linting warning
7c97d40
to
ccedd3b
Compare
* Using invenio_base.utils.entry_points for entry_points
72d0404
to
448a62c
Compare
❤️ Thank you for your contribution!
Description
Please test thoroughly, as I wasn’t able to run the full job in my dev environment.
Errors that is being fixed:
Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Frontend
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: