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

📝 Highlight also line 3 in section Select from the Database #1078

Closed
wants to merge 2 commits into from

Conversation

clelange
Copy link

This line also needs to be changed w.r.t. the above since the select import is added/needed.

This line also needs to be changed w.r.t. the above
@github-actions github-actions bot added the docs Improvements or additions to documentation label Aug 21, 2024
Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Hi, thanks a lot for the contribution! We always appreciate PR's improving the documentation 🙏

In this particular case, I'm unsure whether we want to highlight the import line. You're right that we do need to import select at this point in the tutorial, but in the example before this one we also don't highlight the import line that imports create_engine. I think it might be fine just focusing on the code block here and not divert attention too much to the imports.

If we do want this change - then I would probably also highlight the import in the previous example.

@svlandeg svlandeg changed the title Highlight also line 3 in section Select from the Database 📝 Highlight also line 3 in section Select from the Database Aug 23, 2024
@clelange
Copy link
Author

Hi @svlandeg -- thanks for your feedback. I had another look and I agree with you, the highlighting should probably not be seen as "diff". I'll close this pull request.

@clelange clelange closed this Aug 23, 2024
@clelange clelange deleted the patch-1 branch August 23, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants