Skip to content

Conversation

vanshuhassija
Copy link
Contributor

What changes were proposed in this pull request?

Add capability to register Stack version

How was this patch tested?

Manually, test cases included

@vanshuhassija
Copy link
Contributor Author

@zRains Can you please help with the review?

@zRains zRains self-requested a review March 27, 2025 11:17
@zRains
Copy link
Contributor

zRains commented Mar 27, 2025

@vanshuhassija Sorry about this, I don’t have much spare time right now. I’ll get to it by the weekend at the latest.

@vanshuhassija vanshuhassija force-pushed the feature/AMBARI-26175-register-version branch from 0fd385c to a8330c2 Compare March 28, 2025 05:56
Copy link
Contributor

@zRains zRains left a comment

Choose a reason for hiding this comment

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

It seems your code contains many any types, which weakens the functionality of TypeScript and makes troubleshooting more difficult. If the type is currently unclear, please try to use unknown instead.

type ModalProps = {
isOpen: boolean
onClose: () => void
onReadVersion: Function
Copy link
Contributor

Choose a reason for hiding this comment

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

Type is vague, please specify.

type="file"
className="py-1"
onChange={() => {
//@ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Please try to avoid using this identifier.

<Modal.Header closeButton>
<Modal.Title>Public Repository Option Disabled</Modal.Title>
</Modal.Header>
<Modal.Body style={{ fontSize: 12 }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please try to avoid using inline styles; you can use Tailwind instead.

/* eslint-disable no-useless-escape */
/* eslint-disable no-unsafe-optional-chaining */
/* eslint-disable @typescript-eslint/ban-ts-comment */
/* eslint-disable @typescript-eslint/no-explicit-any */
Copy link
Contributor

Choose a reason for hiding this comment

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

Please try to avoid using these identifiers; if certain factors necessitate their use, please specify the reasons.

Copy link

@nikita15p nikita15p left a comment

Choose a reason for hiding this comment

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

Please fix errors in test files Register.test.tsx as mentioned in the CI pipeline
eg
[INFO] src/tests/StackVersions/Register.test.tsx(47,17): error TS2769: No overload matches this call. [INFO] Overload 1 of 2, '(props: RouterProps): Router', gave the following error. [INFO] Type 'MemoryHistory' is missing the following properties from type 'History<unknown>': length, goBack, goForward [INFO] Overload 2 of 2, '(props: RouterProps, context: any): Router', gave the following error.

@OneTian1211
Copy link

Here's the English translation:

The RPM package names compiled by Ambari for Bigtop and their component names during yum installation are inconsistent with the official Bigtop binary RPM names. This situation forces users to only use Bigtop RPM packages compiled by Ambari. For example, the official Bigtop package name is 'hadoop-3.3.6-1.el7.x86_64.rpm', while the Ambari-compiled Bigtop RPM package name is 'hadoop_3_3_0-3.3.6-1.el8.x86_64.rpm'.

@vanshuhassija vanshuhassija force-pushed the feature/AMBARI-26175-register-version branch from a8330c2 to 37b62ed Compare April 19, 2025 20:39
@vanshuhassija
Copy link
Contributor Author

@zRains I have addressed some of the typescript issues. I intend to address more in future PRs before finalising this project. Merging this PR can also open the feature to community to make some changes to the implemented feature.

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.

4 participants