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 gitlab gitkeep issue #3199

Merged
merged 15 commits into from
Jan 16, 2025
Merged

Fix gitlab gitkeep issue #3199

merged 15 commits into from
Jan 16, 2025

Conversation

akshay-gupta7
Copy link
Contributor

@akshay-gupta7 akshay-gupta7 commented Nov 4, 2024

Why does this PR exist?

Fixes #3160

When gitlab creates empty folders/directories, it creates an empty .gitkeep files.
When our plugin users push tokens to those directories, they found that the gitkeep files still exist, thereby interfering with their token pipelines.

What does this pull request do?

This PR adds ability to remove gitkeep files in non empty directories, the next time user pushes a commit to Gitlab

Testing this change

  • Push a commit to an existing directory with gitkeep files
  • The commit should also take care of removing those redundant gitkeep files

Additional Notes (if any)

Copy link

changeset-bot bot commented Nov 4, 2024

🦋 Changeset detected

Latest commit: e7dcc35

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tokens-studio/figma-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Nov 4, 2024

⤵️ 📦 ✨ The artifact was successfully created! Want to test it? Download it here 👀 🎁

try {
await this.gitlabClient.Commits.create(this.projectId, this.branch, 'Deleting unnecessary .gitkeep files', gitkeepDeletions);
} catch (e) {
console.error('Failed to delete .gitkeep files:', e);

Check warning

Code scanning / ESLint

disallow the use of `console`

Unexpected console statement.
{ action: 'delete', filePath: gitkeepPath },
]);
} catch (e: any) {
console.error(`Failed to delete .gitkeep: ${e}`);

Check warning

Code scanning / ESLint

disallow the use of `console`

Unexpected console statement.
Copy link
Contributor

github-actions bot commented Nov 4, 2024

Commit SHA:2404d70164d44aa044c8807c9348e6cc33a942b8

Test coverage results 🧪

Code coverage diff between base branch:main and head branch: fix-gitlab-gitkeep-issue 
Status File % Stmts % Branch % Funcs % Lines
🔴 total 67.02 (-0.07) 58.05 (-0.03) 63.89 (0.02) 67.4 (-0.07)
🔴 packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts 53.73 (-6.27) 45.74 (-2.03) 54.16 (2.16) 54.16 (-6.21)

Copy link
Contributor

github-actions bot commented Nov 4, 2024

Commit SHA:2404d70164d44aa044c8807c9348e6cc33a942b8
Current PR reduces the test coverage percentage by 1 for some tests

@rbosker rbosker added bug Something isn't working sync gitlab labels Dec 9, 2024
@keeganedwin
Copy link
Collaborator

Tested the fix and it works

@akshay-gupta7 akshay-gupta7 self-assigned this Jan 10, 2025
@akshay-gupta7 akshay-gupta7 requested a review from six7 January 10, 2025 09:00
@akshay-gupta7 akshay-gupta7 marked this pull request as ready for review January 10, 2025 09:01

try {
const response = await this.gitlabClient.Commits.create(
await this.gitlabClient.Commits.create(
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this have a side-effect? we are kind of returning if it was a successful push, now we are depending on the gitkeep stuff. but if its a single file sync we should never even call the gitkeep action

@akshay-gupta7 akshay-gupta7 requested a review from six7 January 16, 2025 11:14
@six7 six7 requested a review from Copilot January 16, 2025 12:57

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts:114

  • The new function 'cleanupGitkeepFiles' should be covered by tests to ensure its correctness.
private async cleanupGitkeepFiles(path: string, branch: string, message: string) {

packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts:289

  • [nitpick] The error message logged here is generic. Consider logging a more specific error message for better debugging.
console.error(err);

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@akshay-gupta7 akshay-gupta7 merged commit daba40f into main Jan 16, 2025
9 of 10 checks passed
@akshay-gupta7 akshay-gupta7 deleted the fix-gitlab-gitkeep-issue branch January 16, 2025 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sync gitlab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with .gitkeep Files and Branch Switching
4 participants