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.

Sign up for GitHub

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

Jump to bottom

Removing SentencePiece Dependency pin as it's no longer needed #1314

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Removing SentencePiece Dependency pin as it's no longer needed #1314

wants to merge 1 commit into from

Conversation

parambole
Copy link
Collaborator

Description

Removing sentencepiece pin as it's no longer needed.

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed.

@gemini-code-assist gemini-code-assist
Copy link

Important

The terms of service for this installation has not been accepted. Please ask the Organization owners to visit the Gemini Code Assist Admin Console to sign it.

@parambole parambole changed the title Removing the Pin as it's no longer needed Removing SentencePiece Dependency pin as it's no longer needed Feb 26, 2025
gobbleturk
gobbleturk approved these changes Feb 26, 2025
View reviewed changes
Copy link
Collaborator

@gobbleturk gobbleturk left a comment

Choose a reason for hiding this comment

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

How about also removing this from requirements.txt?

@parambole parambole force-pushed the parambole/dependency_update branch from 0a9895a to 9f2a099 Compare February 26, 2025 19:16
@parambole
Copy link
Collaborator Author

How about also removing this from requirements.txt?

Done! Also we have an effort to remove requirements.txt and keep a single source of truth. Will followup with you on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gobbleturk gobbleturk gobbleturk approved these changes

@khatwanimohit khatwanimohit Awaiting requested review from khatwanimohit khatwanimohit is a code owner

@bvandermoon bvandermoon Awaiting requested review from bvandermoon bvandermoon is a code owner

@vipannalla vipannalla Awaiting requested review from vipannalla vipannalla is a code owner

@RissyRan RissyRan Awaiting requested review from RissyRan RissyRan is a code owner

@richjames0 richjames0 Awaiting requested review from richjames0 richjames0 is a code owner

@rni418 rni418 Awaiting requested review from rni418 rni418 is a code owner

@gagika gagika Awaiting requested review from gagika gagika is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@parambole @gobbleturk