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

refactor: parsing image data for versions before 24.12 #2817

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

agatha197
Copy link
Contributor

@agatha197 agatha197 commented Nov 5, 2024

Changes:
Refactored image handling similar to #2785 and #2795 for versions before 24.12. Before 24.12, most data parsing is handled on the frontend.

Key changes:

  • Consolidated separate tag components (BaseImageTags, ConstraintTags, LangTags) into a single ImageTags component
  • Simplified image metadata extraction by using string operations instead of complex parsing
  • Removed redundant language column and consolidated information into Tags column
  • Added new getTags utility function to consistently parse and format image tag information
  • Updated image search functionality to search through tag keys and values

Impact:

  • Cleaner, more consistent display of image metadata in the UI
  • More maintainable code structure for handling image tags
  • Improved search capabilities across image metadata

Screenshots:

image.png

What to check:
Data parsing is the same as 24.12.

Checklist:

  • Mention to the original issue
  • Documentation
  • Minium required manager version: manager < 24.12
  • Specific setting for review: 10.100.64.15
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after: will be handled in another stack

Copy link

graphite-app bot commented Nov 5, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added the size:L 100~500 LoC label Nov 5, 2024
@agatha197 agatha197 changed the title refactor: parsing image data before 24.12 refactor: parsing image data for versions before 24.12 Nov 5, 2024
@agatha197 agatha197 added this to the 24.09 milestone Nov 5, 2024
@agatha197 agatha197 force-pushed the refactor/use-filterEmptyItem branch from a7d80b1 to 226486e Compare November 5, 2024 14:39
@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch from 0af5704 to c19e56e Compare November 5, 2024 14:39
@agatha197 agatha197 added type:refactor Refactoring current implementation. urgency:4 As soon as feasible, implementation is essential. labels Nov 5, 2024
@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch from c19e56e to 7cb84bf Compare November 6, 2024 02:50
@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch from 7cb84bf to 6480068 Compare November 6, 2024 02:53
@yomybaby yomybaby changed the base branch from refactor/use-filterEmptyItem to graphite-base/2817 November 6, 2024 04:40
@yomybaby yomybaby force-pushed the refactor/parsing-image-data-before-2412 branch from 6480068 to 9500252 Compare November 6, 2024 04:41
@yomybaby yomybaby changed the base branch from graphite-base/2817 to main November 6, 2024 04:42
@yomybaby yomybaby force-pushed the refactor/parsing-image-data-before-2412 branch from 9500252 to 3708a1b Compare November 6, 2024 04:42
Copy link

github-actions bot commented Nov 6, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
5.02% (-0.01% 🔻)
367/7314
🔴 Branches
4.41% (+0.02% 🔼)
221/5013
🔴 Functions
2.94% (-0% 🔻)
71/2411
🔴 Lines
4.92% (-0.01% 🔻)
352/7153
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴 helper/index.tsx
50.97% (-1.35% 🔻)
47.22%
39.47% (-1.07% 🔻)
52.24% (-1.61% 🔻)
🔴 hooks/index.tsx
37.96% (-4.67% 🔻)
24.39% (-3.39% 🔻)
19.51% (-1.54% 🔻)
37.31% (-4.7% 🔻)

Test suite run success

100 tests passing in 13 suites.

Report generated by 🧪jest coverage report action from 4543a32

@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch from 3708a1b to 1c990ef Compare November 6, 2024 04:59
@agatha197 agatha197 mentioned this pull request Nov 6, 2024
6 tasks
@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch 3 times, most recently from 775b8be to f81d39c Compare November 6, 2024 08:00
react/src/helper/index.tsx Outdated Show resolved Hide resolved
react/src/components/AliasedImageDoubleTags.tsx Outdated Show resolved Hide resolved
@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch from f81d39c to afaa410 Compare November 7, 2024 05:21
@agatha197 agatha197 requested a review from yomybaby November 7, 2024 05:21
@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch from afaa410 to a4f70a9 Compare November 7, 2024 05:26
@yomybaby yomybaby force-pushed the refactor/parsing-image-data-before-2412 branch from a4f70a9 to 3a506af Compare November 7, 2024 09:04
@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch 3 times, most recently from 744604e to 7c8e7da Compare November 11, 2024 05:29
react/src/hooks/index.tsx Show resolved Hide resolved
react/src/components/ImageTags.tsx Show resolved Hide resolved
react/src/components/ImageList.tsx Show resolved Hide resolved
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM

@agatha197 agatha197 force-pushed the refactor/parsing-image-data-before-2412 branch 2 times, most recently from c8306dd to 6993177 Compare November 12, 2024 08:30
Copy link

graphite-app bot commented Nov 12, 2024

Merge activity

**Changes:**
Refactored image handling similar to #2785 and #2795 for versions before 24.12. Before 24.12, most data parsing is handled on the frontend.

Key changes:
- Consolidated separate tag components (BaseImageTags, ConstraintTags, LangTags) into a single ImageTags component
- Simplified image metadata extraction by using string operations instead of complex parsing
- Removed redundant language column and consolidated information into Tags column
- Added new getTags utility function to consistently parse and format image tag information
- Updated image search functionality to search through tag keys and values

**Impact:**
- Cleaner, more consistent display of image metadata in the UI
- More maintainable code structure for handling image tags
- Improved search capabilities across image metadata

**Screenshots:**

![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/2HueYSdFvL8pOB5mgrUQ/483e263b-88bc-4d5e-8a22-d3e219695f76.png)

**What to check:**
Data parsing is the same as 24.12.

**Checklist:**
- [ ] Mention to the original issue
- [ ] Documentation
- [x] Minium required manager version: manager < 24.12
- [x] Specific setting for review: 10.100.64.15
- [x] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after: will be handled in another stack
@yomybaby yomybaby force-pushed the refactor/parsing-image-data-before-2412 branch from 6993177 to 4543a32 Compare November 12, 2024 08:55
@graphite-app graphite-app bot merged commit 4543a32 into main Nov 12, 2024
8 checks passed
@graphite-app graphite-app bot deleted the refactor/parsing-image-data-before-2412 branch November 12, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L 100~500 LoC type:refactor Refactoring current implementation. urgency:4 As soon as feasible, implementation is essential.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants