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

Task/WP-66: Refactored some variables and props to include proptypes #876

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

asimregmi
Copy link
Contributor

Overview

Some variable names and props in the DataFiles components weren't descriptive enough. This change fixes that to the extent without changing the original intention of the code. Also some of the variables needed destructuring which this achieves.

Related

Changes

  • Changed some variables names
  • Updated propTypes where missing
  • Destructured some objects

Testing

  1. Go to Data Files and making sure functionalities work as expected without breaking anything

UI

No visible UI change expected from this PR

Notes

@asimregmi asimregmi requested review from shayanaijaz and removed request for shayanaijaz October 5, 2023 21:07
@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Merging #876 (66af0e2) into main (67f4e75) will decrease coverage by 0.02%.
Report is 2 commits behind head on main.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #876      +/-   ##
==========================================
- Coverage   63.00%   62.99%   -0.02%     
==========================================
  Files         427      427              
  Lines       12191    12197       +6     
  Branches     2504     2506       +2     
==========================================
+ Hits         7681     7683       +2     
- Misses       4303     4306       +3     
- Partials      207      208       +1     
Flag Coverage Δ
javascript 68.67% <85.71%> (-0.04%) ⬇️
unittests 57.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ataFilesProjectMembers/DataFilesProjectMembers.jsx 56.00% <ø> (ø)
...ts/DataFiles/DataFilesSidebar/DataFilesSidebar.jsx 87.17% <100.00%> (ø)
...ents/DataFiles/DataFilesStatus/DataFilesStatus.jsx 71.15% <87.50%> (+0.56%) ⬆️
...ataFilesSystemSelector/DataFilesSystemSelector.jsx 75.67% <75.00%> (ø)

... and 2 files with indirect coverage changes

@asimregmi asimregmi marked this pull request as ready for review October 5, 2023 21:12
@asimregmi asimregmi changed the title Refactored some variables and props to include proptypes Task/WP-66: Refactored some variables and props to include proptypes Oct 5, 2023
Copy link
Contributor

@edmondsgarrett edmondsgarrett left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -286,6 +286,7 @@ const DataFilesProjectMembers = ({
};

DataFilesProjectMembers.propTypes = {
projectId: PropTypes.string,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Without projectId, was this broken previously. This seems more than refactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't broken but included it for typechecking. I can remove it if its out of scope for this PR.

@chandra-tacc chandra-tacc merged commit b31a09d into main Oct 9, 2023
5 of 6 checks passed
@chandra-tacc chandra-tacc deleted the WP-66/Refactor-DataFiles-components branch October 9, 2023 18:38
chandra-tacc pushed a commit that referenced this pull request Oct 9, 2023
…876)

* Refactored some variables and props to include proptypes

* fixed linting errors
chandra-tacc added a commit that referenced this pull request Oct 9, 2023
chandra-tacc added a commit that referenced this pull request Oct 10, 2023
* WA-314: Input file fixes for hidden and fixed

* Task/WP-66: Refactored some variables and props to include proptypes (#876)

* Refactored some variables and props to include proptypes

* fixed linting errors

* WP-299: Add Data Files button dropdown needs minor adjustment in alignment (#878)

* small UI change to align dropdown

* linting issues fixed

---------

Co-authored-by: Chandra Y <[email protected]>

---------

Co-authored-by: Asim Regmi <[email protected]>
chandra-tacc added a commit that referenced this pull request Oct 10, 2023
* WA-314: Input file fixes for hidden and fixed

* Task/WP-66: Refactored some variables and props to include proptypes (#876)

* Refactored some variables and props to include proptypes

* fixed linting errors

* WP-299: Add Data Files button dropdown needs minor adjustment in alignment (#878)

* small UI change to align dropdown

* linting issues fixed

---------

Co-authored-by: Chandra Y <[email protected]>

---------

Co-authored-by: Asim Regmi <[email protected]>
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.

3 participants