Skip to content

[js] Adding logger documentation #1321

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

Merged
merged 8 commits into from
Apr 20, 2023
Merged

Conversation

TamsilAmani
Copy link
Contributor

@TamsilAmani TamsilAmani commented Feb 28, 2023

Description

Documentation on how to use Logging feature in JS binding. Formatting documentation for Ruby and JS.

Motivation and Context

SeleniumHQ/selenium#10944

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

@netlify
Copy link

netlify bot commented Feb 28, 2023

Deploy Preview for selenium-dev ready!

Name Link
🔨 Latest commit 532a104
🔍 Latest deploy log https://app.netlify.com/sites/selenium-dev/deploys/6441297d0c909900088584d5
😎 Deploy Preview https://deploy-preview-1321--selenium-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@diemol diemol requested a review from titusfortner February 28, 2023 10:04
@titusfortner
Copy link
Member

I think we should put this whole page in a tab rather than bullet points per language. What do you think?

@titusfortner
Copy link
Member

This is formatted incorrectly. We changed how tabs need to display code with the latest Docsy update, needs to be text=true instead of code=false

And can we move & update the {{< alert-content >}} lines to go into the tabs that don't have information?

Thanks!

@TamsilAmani
Copy link
Contributor Author

Thank you for the help regarding formatting @titusfortner. I have pushed the changes.

Copy link
Member

@titusfortner titusfortner left a comment

Choose a reason for hiding this comment

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

A few things I'm not clear on...

Also, can we get an example of what actual output looks like based on example code?

@TamsilAmani
Copy link
Contributor Author

A few things I'm not clear on...

Also, can we get an example of what actual output looks like based on example code?

Can you elaborate this a little please @titusfortner? Do you want to show outputs for different languages in their respective tabs along with a working sample code?

@titusfortner
Copy link
Member

Do you want to show outputs for different languages in their respective tabs along with a working sample code

I meant like just a line showing actual output. Like this:

https://github.com/SeleniumHQ/seleniumhq.github.io/blob/trunk/website_and_docs/content/documentation/webdriver/troubleshooting/logging.en.md?plain=1#L44-L45

Copy link
Member

@titusfortner titusfortner left a comment

Choose a reason for hiding this comment

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

A few thoughts to make it more language-agnostic. I can update the Ruby tab things after the next pass. Thanks!

@TamsilAmani
Copy link
Contributor Author

Please have a look @titusfortner. Let me know if any changes are to be made. This PR only adds JS and ruby sections. For other bindings I'll make separate PRs.

Copy link
Member

@titusfortner titusfortner 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! A couple minor tweaks.

@TamsilAmani
Copy link
Contributor Author

Changes pushed.

@titusfortner
Copy link
Member

Oh, and this probably should use markdown instead of html.

https://www.selenium.dev/documentation/about/style/#using-markdown-in-a-tab

need to use {{% tab header="Java" %}} instead of {{< tab header="Java" >}}

@TamsilAmani
Copy link
Contributor Author

Ah! Thank you for that link. I was just wondering how to use MD instead of HTML and was all over the internet for the solution. Will push the changes soon.

@titusfortner
Copy link
Member

lol, yeah, it isn't obvious. I only know because I was paying attention to those specific changes when they were made (we didn't used to be able to do text in the tabs of any kind).

@diemol
Copy link
Member

diemol commented Apr 4, 2023

I guess we need to have the files in the other languages in order to merge this.

@TamsilAmani
Copy link
Contributor Author

@diemol So should I push the docs for Java and Python in this same branch? (Also I haven't explored how things are done in C# and Kotlin hence that might take time)

@titusfortner
Copy link
Member

No, he means the other translated md files. Just copy paste what you have over and our amazing translators will do the rest.

@TamsilAmani
Copy link
Contributor Author

@titusfortner The alert-content is not working with markdown style (%). So I have left the tabs which use alerts to still use html (<>). The markdown style can be done when these tabs are filled with their content.

@diemol I have added other language file.

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

Thank you, @TamsilAmani!

@diemol diemol merged commit 6bb3ef1 into SeleniumHQ:trunk Apr 20, 2023
selenium-ci added a commit that referenced this pull request Apr 20, 2023
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.

3 participants