Get the config directory from $XDG_CONFIG_HOME if it is set#1769
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly implements support for the $XDG_CONFIG_HOME environment variable for locating the toolbox configuration directory. My review includes a suggestion to refactor the shell command for setting the configuration path to be more idiomatic and robust by using parameter expansion. This improves code clarity and maintainability.
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 11s |
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 08s |
debarshiray
left a comment
debarshiray
left a comment
There was a problem hiding this comment.
Thanks for working on this, @ymcx and my apologies for the delay! The overall code changes look good to me.
Could you please use your real name and email address in the Git commit message, and squash both the commits together (because they are one single logical change)?
No worries.
Which email address do you want me to use? Do I need to use the real address that is linked to this account, or will any address work that can be used to contact me? |
The real name and email address requirement is for tracking copyright holders in case a licensing issue crops up. So, any email address that can be used to contact you is enough. :) Also, note, for the sake of completeness: The Projects as diverse as GCC, GnuPG, Linux, Moby and Podman don't allow anonymous or pseudonymous contributions. |
ymcx
force-pushed
the
respect-xdg-config-home
branch
from
0f5e62a to
f999655
Compare
|
Will this work? I don't think I can sign the commit with this email address.
Thanks for the explanation! I tried looking for the related guidelines but didn't find them elsewhere. |
... instead of hard coding ~/.config. This is specified in the XDG Base Directory Specification [1]. [1] https://specifications.freedesktop.org/basedir/latest/ containers#1766
debarshiray
force-pushed
the
respect-xdg-config-home
branch
from
f999655 to
b324146
Compare
debarshiray
left a comment
debarshiray
left a comment
There was a problem hiding this comment.
Thanks for the updates, @ymcx ! If it's a valid email address that can be used to contact you, then it's fine. :)
I took the liberty to rebase against main and added a link to the XDG Base Directory specification for future reference.
The changes seem to work well in my testing. Let's merge once the CI finishes.
|
Unrelated to the main goal of this pull request, but somewhat related, two things came to my notice. First, there are some instances of It would be a cosmetic change because Second, we need to migrate away from @ymcx does any of those interest you? :) |
These are odd: ... and we started seeing them recently in #1798 I am going to take a leap of faith and ignore this failure on the assumption that it's something to do with the Software Factory infrastructure, because everything was fine a week ago and nothing relevant to these tests changed since then. |
|
Thank you for your contribution!
Let me know. :) |
2 participants
Closes #1766
If the $XDG_CONFIG_HOME environment variable is set, we should use it instead of always falling back to $HOME/.config