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

Dice game example #173

Merged
merged 4 commits into from
Oct 27, 2023
Merged

Conversation

marcdel
Copy link
Contributor

@marcdel marcdel commented May 18, 2023

Adds a dice game example in service of this documentation pull request: open-telemetry/opentelemetry.io#2685

@tsloughter
Copy link
Member

I assume it is an accident that this has 919 files? :)

@benregn
Copy link

benregn commented May 31, 2023

It's because Phoenix now vendors all the icons from https://heroicons.com/ for newly generated projects. I assume they could be trimmed back since probably only a handful of icons (maybe?) are used in this project 🙂

@tsloughter
Copy link
Member

None should be used in this project, so they can all be removed.

@lessless
Copy link

hey @benregn 👋

Thanks for all the work you put in. I want to suggest rewriting the example in Mix plus Cowboy.
Phoenix does carry too much baggage that takes away attention from the core topic.

@benregn
Copy link

benregn commented Jun 14, 2023

@lessless I'm not the author of this PR but thanks for the suggestion.

However in my opinion, an example using Phoenix is probably more helpful because it's used more in the wild.

Copy link
Member

@tsloughter tsloughter left a comment

Choose a reason for hiding this comment

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

I'll merge once the unnecessary files, like icons are removed.

@tsloughter
Copy link
Member

@marcdel ping

@marcdel marcdel force-pushed the dice-game-example branch 2 times, most recently from 7469193 to 4c19577 Compare October 6, 2023 21:21
@marcdel
Copy link
Contributor Author

marcdel commented Oct 6, 2023

@tsloughter sorry, I totally spaced on this. I regenerated the app without all the html stuff which simplifies it significantly. Happy to add metrics examples as well, but probably as a separate PR.

Copy link
Member

@tsloughter tsloughter left a comment

Choose a reason for hiding this comment

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

Looks good! Just had one change to match the other implementations of the example in other languages.

@tsloughter
Copy link
Member

Thanks! Sounds good with having another PR.

@marcdel
Copy link
Contributor Author

marcdel commented Oct 17, 2023

@tsloughter boop! rebased this guy, so should be good to go.

@tsloughter tsloughter merged commit c7edb8a into open-telemetry:main Oct 27, 2023
22 checks passed
@marcdel marcdel deleted the dice-game-example branch October 30, 2023 21:58
@yordis yordis mentioned this pull request Dec 12, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants