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

Adds a clarification around artifact types. #677

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

katjacksonWB
Copy link
Contributor

Description

Adds a clarification around artifact types based on customer feedback.

Ticket

Does this PR fix an existing issue? If yes, provide a link to the ticket here:

Checklist

Check if your PR fulfills the following requirements. Put an X in the boxes that apply.

  • Files I edited were previewed on my local development server with yarn start. My changes did not break the local preview.
  • Build (yarn docusaurus build) was run locally and successfully without errors or warnings.
  • I merged the latest changes from main into my feature branch before submitting this PR.

@katjacksonWB katjacksonWB requested a review from a team as a code owner May 21, 2024 22:59
@katjacksonWB katjacksonWB requested a review from noaleetz May 22, 2024 18:09
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-05-22 at 9 50 49 PM

I think it could be helpful to explicitly connect the image with the code snippet and show the relationship in a diagram. I attached a sketch I made, maybe it's worth getting a more official version with Raven's help?

Copy link
Contributor

Choose a reason for hiding this comment

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

@noaleetz I have updated this PR for Hugo but using the exact same image and text that Kathryn came up with previously. Do you still think that an image similar to this one is better?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey! Yes - I think sharing an image that demonstrates:

  • how type is defined in an artifact
  • how that will be represented in lineage
    is important, as we've had users ask how this field is set, and what it impacts in the artifact UI

Copy link
Contributor

@noaleetz noaleetz left a comment

Choose a reason for hiding this comment

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

sorry I forgot I never submitted the actual review - left one more comment on image!

Copy link

cloudflare-workers-and-pages bot commented Oct 3, 2024

Deploying docodile with  Cloudflare Pages  Cloudflare Pages

Latest commit: 13f6fb1
Status: ✅  Deploy successful!
Preview URL: https://cb0d4bb6.docodile.pages.dev
Branch Preview URL: https://artifacts-landing.docodile.pages.dev

View logs

Copy link
Contributor

github-actions bot commented Oct 3, 2024

Images automagically compressed by Calibre's image-actions

Compression reduced images by 45.3%, saving 130.03 KB.

Filename Before After Improvement Visual comparison
static/images/artifacts/artifacts_dropdown.png 286.90 KB 156.87 KB -45.3% View diff

409 images did not require optimisation.

Copy link
Contributor

github-actions bot commented Nov 6, 2024

Summary

Status Count
🔍 Total 15
✅ Successful 2
⏳ Timeouts 0
🔀 Redirected 0
👻 Excluded 13
❓ Unknown 0
🚫 Errors 0
Full Github Actions output

Copy link

cloudflare-workers-and-pages bot commented Feb 6, 2025

Deploying docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8b20a01
Status: ✅  Deploy successful!
Preview URL: https://30361155.docodile.pages.dev
Branch Preview URL: https://artifacts-landing.docodile.pages.dev

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants