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

SALTO-7363: Removing unused exports from all non-adapter packages #7226

Merged
merged 2 commits into from
Feb 12, 2025

Conversation

yelly
Copy link
Contributor

@yelly yelly commented Feb 10, 2025

Preparation for enabling knip unused exports rules.


Additional context for reviewer


Release Notes:
None.


User Notifications:
None.

@yelly yelly marked this pull request as ready for review February 10, 2025 09:38
@yelly yelly requested review from ori-moisis and a team February 10, 2025 09:40
@coveralls
Copy link

Coverage Status

coverage: 93.649% (+0.01%) from 93.637%
when pulling fec2c4e on yelly:SALTO-7363-core
into 8aa4c52 on salto-io:main.

@@ -6,4 +6,3 @@
* CERTAIN THIRD PARTY SOFTWARE MAY BE CONTAINED IN PORTIONS OF THE SOFTWARE. See NOTICE FILE AT https://github.com/salto-io/salto/blob/main/NOTICES
*/
export { createClientDefinitions } from './clients'
export { PAGINATION as pagination } from './pagination'
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't review the rest of the PR, but please don't remove things from service placeholder as this serves as a placeholder to create a new adapter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the service placeholder adapter from this PR, we will work out how to handle it separately.

@@ -29,6 +29,9 @@ import {
getSalesforceClient,
} from './helpers/salesforce'
import * as callbacks from '../src/callbacks'
// Importing getCredentialsFromUser since this file is the only place it's used but it's only referenced by name for spying
// eslint-disable-next-line @typescript-eslint/no-unused-vars
import { getCredentialsFromUser as _getCredentialsFromUser } from '../src/callbacks'
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need both the lint disable and the as thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, not using a declared variable not starting in an underscore is a TS warning, not an eslint one.
If we want to not do the rename we would need to use a @ts-ignore directive instead of an eslint-disable directive which is more dangerous.

@yelly yelly merged commit 5ec5ea6 into salto-io:main Feb 12, 2025
62 checks passed
@yelly yelly deleted the SALTO-7363-core branch February 12, 2025 20:27
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