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

[UISACQCOMP-175] Add initial stripes-acq-components #42

Merged
merged 7 commits into from
Jan 2, 2024

Conversation

ncovercash
Copy link
Member

Copy link

github-actions bot commented Dec 21, 2023

Jest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit ec79d83. ± Comparison against base commit bee3647.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 21, 2023

BigTest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit ec79d83. ± Comparison against base commit bee3647.

♻️ This comment has been updated with latest results.

Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

@ncovercash, lint whines about "Unexpected any." in some files (AcqCheckboxFilter) but not others with the same declaration (AcqList/SingleSearchForm). Is that an inconsistency that should be fixed or do I just now know much about TS?

Most likely it's the latter, in which case this is fine. But I'd like to understand why a declaration that appears to be the same results in different behavior.

@ncovercash
Copy link
Member Author

@ncovercash, lint whines about "Unexpected any." in some files (AcqCheckboxFilter) but not others with the same declaration (AcqList/SingleSearchForm). Is that an inconsistency that should be fixed or do I just now know much about TS?

Not sure...my best guess is that we might be hitting some kind of limit in eslint, since there are so many of these in stripes-types. Thinking about it, due to the nature of this project, we should disable this linting rule IMO; I'll implement that.

@ncovercash ncovercash requested a review from zburke January 2, 2024 19:44
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

Yay! 🎉 More lint complaints are now visible now that you handled all those 'no any' errors! 😒 Now we have lots of ... is defined but never used in files like acq-components/lib/constants/constants.d.ts. Same story as before: this complaint appears in some files but not in others that appear to have identical declarations.

Should we similarly ignore this complaint while developing these types?

@ncovercash
Copy link
Member Author

Yep, this should be ignored too. It's not a top-level export which is why I think we're seeing the error (you'd have to do constants.FUND_DISTR_TYPE.percent, for example).

Copy link

sonarqubecloud bot commented Jan 2, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@ncovercash ncovercash requested a review from zburke January 2, 2024 20:24
@ncovercash
Copy link
Member Author

should be good now!

@ncovercash ncovercash merged commit 6efb120 into master Jan 2, 2024
6 checks passed
@ncovercash ncovercash deleted the UISACQCOMP-175 branch January 2, 2024 21:19
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.

2 participants