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

Devan/eng 1244 artifactskeyname #17030

Merged
merged 8 commits into from
Feb 7, 2025
Merged

Devan/eng 1244 artifactskeyname #17030

merged 8 commits into from
Feb 7, 2025

Conversation

devangrose
Copy link
Contributor

@devangrose devangrose commented Feb 6, 2025 •
edited
Loading

adds /artifacts/key

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

@github-actions github-actions bot added the ui-replatform Related to the React UI rewrite label Feb 6, 2025
devinvillarosa
devinvillarosa reviewed Feb 7, 2025
View reviewed changes
ui-v2/src/components/artifacts/artifact-card.tsx Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Why is key used rather than id? (even for the original ui/)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keys are a one-to-many to artifact IDs. They're a grouping mechanism to show artifact history.

devinvillarosa
devinvillarosa approved these changes Feb 7, 2025
View reviewed changes
Copy link
Contributor

@devinvillarosa devinvillarosa left a comment

Choose a reason for hiding this comment

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

LGTM

devangrose and others added 3 commits February 7, 2025 14:53
@devangrose @devinvillarosa
Update ui-v2/src/components/artifacts/key/timeline/timelineRow.tsx
9523973 
Co-authored-by: Devin Villarosa <102188207+devinvillarosa@users.noreply.github.com>
@devangrose @devinvillarosa
Update ui-v2/src/routes/artifacts/key.$key.tsx
615ab9c 
Co-authored-by: Devin Villarosa <102188207+devinvillarosa@users.noreply.github.com>
@devangrose
Merge branch 'main' into devan/eng-1244-artifactskeyname
4b42652
@devangrose devangrose merged commit 0794427 into main Feb 7, 2025
7 checks passed
@devangrose devangrose deleted the devan/eng-1244-artifactskeyname branch February 7, 2025 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devinvillarosa devinvillarosa devinvillarosa approved these changes

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

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

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

Assignees
No one assigned
Labels
ui-replatform Related to the React UI rewrite
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@devangrose @devinvillarosa