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

fix: add F flag to remove unused imports #52649

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

artem1205
Copy link
Collaborator

What

add F flag to remove unused imports

How

add F flag to remove unused imports

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@artem1205 artem1205 requested a review from aaronsteers January 30, 2025 10:48
@artem1205 artem1205 self-assigned this Jan 30, 2025
Copy link

vercel bot commented Jan 30, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 3, 2025 10:00pm

@natikgadzhi
Copy link
Contributor

natikgadzhi commented Jan 30, 2025

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (8d7ec6c)

@natikgadzhi
Copy link
Contributor

Running formatters to see what the diff would be :)

@natikgadzhi
Copy link
Contributor

I personally think this is a good idea, so if CI is green-adjacent, we should prob merge this.

@artem1205
Copy link
Collaborator Author

hey @natikgadzhi ,

CI is green-adjacent, we should prob merge this.

Ci will never be green, since it is expected to bump version whenever any change in code occurs.

My proposal is to merge with the ca7ac53 only and gradually fix the connectors/ci packages when we really work on them, wdyt?

@aaronsteers
Copy link
Collaborator

aaronsteers commented Feb 21, 2025

hey @natikgadzhi ,

CI is green-adjacent, we should prob merge this.

Ci will never be green, since it is expected to bump version whenever any change in code occurs.

My proposal is to merge with the ca7ac53 only and gradually fix the connectors/ci packages when we really work on them, wdyt?

@artem1205 - I believe the latest CI workflows should keep code checks separate from version checks and other "pre-release" checks. If you want to recreate/rerun against the latest master branch, it should be an cleaner CI picture to grok.

Per this merge from last week:

cc @natikgadzhi

@artem1205 artem1205 force-pushed the artem1205/precommit-fix-unused-imports branch from 23320d1 to a6a4f6b Compare February 23, 2025 16:02
@octavia-squidington-iii octavia-squidington-iii removed the area/connectors Connector related issues label Feb 23, 2025
@artem1205
Copy link
Collaborator Author

artem1205 commented Feb 23, 2025

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (bcc0e94)

@natikgadzhi
Copy link
Contributor

From spot checking unit tests, this looks good to me. It’s a linter run so I’m comfortable. I think we can resolve conflicts and merge.

…mports' into artem1205/precommit-fix-unused-imports

# Conflicts:
#	airbyte-integrations/connectors/source-klaviyo/source_klaviyo/source.py
#	airbyte-integrations/connectors/source-klaviyo/unit_tests/integration/test_profiles.py
#	airbyte-integrations/connectors/source-klaviyo/unit_tests/test_streams.py
#	airbyte-integrations/connectors/source-railz/components.py
#	airbyte-integrations/connectors/source-typeform/unit_tests/test_form_id_partition_router.py
@artem1205
Copy link
Collaborator Author

artem1205 commented Mar 3, 2025

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (2a264cf)

Signed-off-by: Artem Inzhyyants <[email protected]>
Signed-off-by: Artem Inzhyyants <[email protected]>
…mports' into artem1205/precommit-fix-unused-imports
Signed-off-by: Artem Inzhyyants <[email protected]>
Signed-off-by: Artem Inzhyyants <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues connectors/destination/astra connectors/destination/duckdb connectors/destination/motherduck connectors/destination/pinecone connectors/destination/timeplus connectors/destination/vectara connectors/source/airtable connectors/source/amazon-ads connectors/source/amazon-seller-partner connectors/source/amazon-sqs connectors/source/amplitude connectors/source/aws-cloudtrail connectors/source/braze connectors/source/chargebee connectors/source/commercetools connectors/source/convex connectors/source/facebook-marketing connectors/source/file connectors/source/gcs connectors/source/github connectors/source/gong connectors/source/google-ads connectors/source/google-analytics-data-api connectors/source/google-drive connectors/source/google-sheets connectors/source/greenhouse connectors/source/hubspot connectors/source/instagram connectors/source/instatus connectors/source/intercom connectors/source/jira connectors/source/klaviyo connectors/source/linkedin-ads connectors/source/linnworks connectors/source/microsoft-sharepoint connectors/source/mixpanel connectors/source/my-hours connectors/source/mysql connectors/source/okta connectors/source/paypal-transaction connectors/source/pinterest connectors/source/railz connectors/source/recharge connectors/source/rss connectors/source/s3 connectors/source/salesforce connectors/source/shopify connectors/source/slack connectors/source/stripe connectors/source/surveymonkey connectors/source/tiktok-marketing connectors/source/typeform connectors/source/xero connectors/source/zendesk-chat connectors/source/zendesk-support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants