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

Adopt EnvironmentVariableCollection in git ext #184413

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Adopt EnvironmentVariableCollection in git ext #184413

merged 3 commits into from
Jun 8, 2023

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Jun 6, 2023

Fixes #184331

image

@Tyriar Tyriar added this to the June 2023 milestone Jun 6, 2023
@Tyriar Tyriar requested a review from lszomoru June 6, 2023 12:48
@Tyriar Tyriar self-assigned this Jun 6, 2023
@lszomoru
Copy link
Member

lszomoru commented Jun 6, 2023

Thanks for the PR. I think that:

  1. We should localize this string
  2. It should be more generic as the functionality also depends on various settings (ex: Git: Enables git experiences in the integrated terminal

@Tyriar
Copy link
Member Author

Tyriar commented Jun 6, 2023

Opps forgot to localize.

It should be more generic as the functionality also depends on various settings (ex: Git: Enables git experiences in the integrated terminal

Could we be more explicit here and tailor strings for each set of values? "git experiences" is very mysterious still. I see git editor which I understand but is the IPC Server one only used for askpass or?

@lszomoru
Copy link
Member

lszomoru commented Jun 6, 2023

Yeah, the two things are the git commit message editor, and askpass integration.

@Tyriar
Copy link
Member Author

Tyriar commented Jun 7, 2023

I improved the message but I'm not sure how to do localization in an extension, I've never done it before and I don't see existing examples in the git extension used outside of the package.json?

@lszomoru
Copy link
Member

lszomoru commented Jun 8, 2023

Localization sample -

return l10n.t('Tag at {0}', this.shortCommit);
}

@Tyriar Tyriar merged commit 911d1c0 into main Jun 8, 2023
6 checks passed
@Tyriar Tyriar deleted the tyriar/184331 branch June 8, 2023 15:52
@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adopt EnvironmentVariableCollection.description in git extension
3 participants