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

Added Zero code image #5409

Merged
merged 5 commits into from
Dec 9, 2024
Merged

Added Zero code image #5409

merged 5 commits into from
Dec 9, 2024

Conversation

mercybassey
Copy link
Contributor

This PR adds an illustration for Open telemetry zero code.

Issue: #5354

@mercybassey mercybassey requested a review from a team as a code owner October 15, 2024 13:27
@mercybassey
Copy link
Contributor Author

Hi @svrnm, I'm having trouble displaying the image. I checked most of the markdown files in the project, and it seems that images are added using this syntax: ![](). However, this method isn't working for me. Here is the output I'm getting:
zero

I'd appreciate your guidance on this.

@svrnm
Copy link
Member

svrnm commented Oct 15, 2024

Do the following:

  • create a new folder zero-code
  • move zero-code.md into that folder and rename the file to index.md
  • move the image into that folder as well

Can you use the SVG instead of the png? This allows us to edit the image in the future much easier and most modern browser render SVGs just fine.

@mercybassey
Copy link
Contributor Author

@svrnm here is my output locally:

zero

Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

Great progress, thanks

@svrnm svrnm added the ux label Oct 15, 2024
@svrnm svrnm linked an issue Oct 15, 2024 that may be closed by this pull request
@mercybassey
Copy link
Contributor Author

@svrnm I have addressed your comments.
zero
Thanks.

@svrnm
Copy link
Member

svrnm commented Oct 16, 2024

@mercybassey thanks, this looks good! I asked @open-telemetry/docs-approvers to take a look as well, so that we have at least one more person to take a look

@mercybassey
Copy link
Contributor Author

@mercybassey thanks, this looks good! I asked @open-telemetry/docs-approvers to take a look as well, so that we have at least one more person to take a look

Great.

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @mercybassey.

@svrnm - I've commented on Slack. PTAL

@svrnm
Copy link
Member

svrnm commented Oct 25, 2024

@mercybassey apologies for the delay here. We need to discuss the details a little bit more before we can get this merged. This will unfortunately not happen before the end of the contribution phase for outreachy. So, assume this as "done" for your application and if you are still interested we can follow up later.

@mercybassey
Copy link
Contributor Author

@mercybassey apologies for the delay here. We need to discuss the details a little bit more before we can get this merged. This will unfortunately not happen before the end of the contribution phase for outreachy. So, assume this as "done" for your application and if you are still interested we can follow up later.

Alright. I’m cool with that. Thanks.

@svrnm
Copy link
Member

svrnm commented Nov 22, 2024

@mercybassey apologies for the delay, we had to skip some of our last meetings, hopefully we will get to this on Monday!

@mercybassey
Copy link
Contributor Author

@mercybassey apologies for the delay, we had to skip some of our last meetings, hopefully we will get to this on Monday!

Great. Thank you.

@svrnm
Copy link
Member

svrnm commented Nov 26, 2024

@mercybassey apologies for the delay, we had to skip some of our last meetings, hopefully we will get to this on Monday!

Great. Thank you.

Unfortunately @chalin is out of office, I will follow up with him to talk this through to give you further feedback. Apologies for the further delay

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

@svrnm - I defer to you on the acceptance of this image as is. Could we at least ensure that it is compatible with dark mode from the outset, which it currently is not:

image

@svrnm
Copy link
Member

svrnm commented Dec 5, 2024

@svrnm - I defer to you on the acceptance of this image as is. Could we at least ensure that it is compatible with dark mode from the outset, which it currently is not:

Thanks! I think this image is a good starting point on which we can iterate. I agree that it should be compatible with dark mode as well, this is accomplished best with a transparent background

@mercybassey please take a look

@mercybassey
Copy link
Contributor Author

@svrnm - I defer to you on the acceptance of this image as is. Could we at least ensure that it is compatible with dark mode from the outset, which it currently is not:

Thanks! I think this image is a good starting point on which we can iterate. I agree that it should be compatible with dark mode as well, this is accomplished best with a transparent background

@mercybassey please take a look

Sure @svrnm. I'll adjust the image and use a transparent background instead. Thanks.

@mercybassey
Copy link
Contributor Author

Hi @svrnm @chalin. I have adjusted the image. Below is the image in dark and light mode.
dark-mode
light-mode

@mercybassey
Copy link
Contributor Author

Hi @svrnm @chalin. I have adjusted the image. Below is the image in dark and light mode. dark-mode light-mode

I noticed that most images have a transparent boundary, so I decided to leave it as is unless instructed otherwise.
example2
example1

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks for the update. The contrast on the text and lines (outside of the boxes) isn't great -- it would require a wee bit more work to fix that and I don't have the time to explore a solution. In any case, this is quite good enough for now!

@svrnm svrnm added this pull request to the merge queue Dec 9, 2024
@svrnm
Copy link
Member

svrnm commented Dec 9, 2024

thank you for your contribution and your patience @mercybassey !

Merged via the queue into open-telemetry:main with commit 2127d75 Dec 9, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add a visualization to "Zero-code instrumentation"
3 participants