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

Update pre-commit hooks to check for headers and white-spaces #1205

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

amahussein
Copy link
Collaborator

Signed-off-by: Ahmed Hussein (amahussein) a@ahussein.me

Contributes to #1158

  • add trailing-whitespace check to the pre-commit hooks to avoid whitespaces at the end of lines
  • add E-O-F fixer to make sure that only a single newline is at the end of each committed file
  • added a script to check that newly added files have the header.
  • pulled the intelli-j idea from RAPIDS plugin and update the the README file. No link is added because the markdown checker could cause the build to fail until a release is made.

Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>

Contributes to NVIDIA#1158

- add trailing-whitespace check to the pre-commit hooks to avoid
  whitespaces at the end of lines
- add E-O-F fixer to make sure that only a single newline is at the end
  of each committed file
- added a script to check that newly added files have the header.
- pulled the intelli-j idea from RAPIDS plugin and update the the README
  file. No link is added because the markdown checker could cause the
build to fail until a release is made.
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
Copy link
Collaborator

@parthosa parthosa left a comment

Choose a reason for hiding this comment

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

Thanks @amahussein. LGTM. Couple of minor comments.

core/README.md Outdated

### Setting up an Integrated Development Environment

Before proceeding with importing spark-rapids into IDEA or switching to a different Spark release
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: spark-rapids-tools

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

core/README.md Outdated
##### Importing the project

To start working with the project in IDEA is as easy as importing the project as a Maven project.
Select the profile used in the mvn command above, e.g. `spark340` for Spark 3.4.0.
Copy link
Collaborator

@parthosa parthosa Jul 19, 2024

Choose a reason for hiding this comment

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

nit: Can we keep all versions to 3.5.0 since that is the default for our repo.

Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
### Setting up an Integrated Development Environment

Before proceeding with importing spark-rapids-tools into IDEA or switching to a different Spark release
profile, execute the install phase with the corresponding `buildver`, e.g. for Spark 3.5.0:
Copy link
Collaborator

@cindyyuanjiang cindyyuanjiang Jul 19, 2024

Choose a reason for hiding this comment

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

nit: "e.g. for Spark 3.5.0", should be "e.g. 350 for Spark 3.5.0"?

Copy link
Collaborator

@cindyyuanjiang cindyyuanjiang left a comment

Choose a reason for hiding this comment

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

Thanks @amahussein! Just a minor nit.

Copy link
Collaborator

@nartal1 nartal1 left a comment

Choose a reason for hiding this comment

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

Thanks @amahussein !

@amahussein amahussein merged commit a445376 into NVIDIA:dev Jul 22, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants