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

Feature/Connectivity: Rework page #135

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Feature/Connectivity: Rework page #135

merged 1 commit into from
Nov 15, 2024

Conversation

amotl
Copy link
Member

@amotl amotl commented Nov 14, 2024

Problem

The Feature » Connectivity page wasn't in a good shape yet. @kneth discovered that recently.

Solution

This patch intends to converge the page into the same layout/shape/style like all the others in the "All Features" section, using the same compact backbone made of "Intro", "Synopsis", and "Learn" subsections.

Preview

https://cratedb-guide--135.org.readthedocs.build/feature/connectivity/

Review

  • Please visit inline comments.
  • We believe the "tabs" approach will not scale, so we diverted using the "dropdowns" approach, currently showing both variants. Please advise.

Details

This refactors a bit of content from a few sibling pages in crate-clients-tools. 1

Demonstrate two different variants for single-page client-/driver-snippets presentation, one using tabs, and another one using dropdowns.

Footnotes

  1. crate-clients-tools might be dissolved in the future, in order to reduce maintenance overhead, so it's a good idea to migrate content gradually.

@amotl amotl added sanding-180 Sanding rough edges. refactoring Changing shape or layout, or moving content around. cross linking Linking to different locations of the documentation. guidance Matters of layout, shape, and structure. labels Nov 14, 2024
@cla-bot cla-bot bot added the cla-signed label Nov 14, 2024
Comment on lines 84 to 88
::::::{tab-set}

:::::{tab-item} C# / Npgsql
For more information see [Npgsql] documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the "tabbed" variant. We think it will not scale, and nesting tabs is also debatable.

Copy link

Choose a reason for hiding this comment

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

I agree, on a small screen (14 in laptop) it is not an optimal solution

Comment on lines 292 to 300
:::::{dropdown} PHP

:::{rubric} HTTP
:::

::::{dropdown} PDO_CRATEDB
The PHP Data Objects (PDO) is a standard PHP extension that defines a common
interface for accessing databases in PHP.
For more information see [CrateDB PDO] documentation.
Copy link
Member Author

Choose a reason for hiding this comment

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

That's the "dropdown" variant, already using nested dropdowns, immediately applicable with PHP and Python for example, as there are many driver variants for them, across two families (HTTP vs. PostgreSQL), which stretches all axes of the system needed to represent those.

We are biased to use this variant, and drop the "tabbed" variant again. wdyt?

Copy link
Member Author

@amotl amotl Nov 14, 2024

Choose a reason for hiding this comment

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

The main question to you here is if you agree to continue with the "dropdown" variant, because the "tabbed" variant doesn't scale well. Thanks!

Copy link

Choose a reason for hiding this comment

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

If we go with the "tabbed" variant, I think we should pick 3-4 items only. So if we want to have all items, the "dropdown" variant might be better.

Copy link
Member Author

@amotl amotl Nov 15, 2024

Choose a reason for hiding this comment

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

Thanks. I tried to present both variants without many initial plans, and just related to a recent comment from your pen that you like "tabbed" approaches of information conveyance, if I got that right.

What do you think how we should proceed on this particular matter?

Copy link
Member Author

@amotl amotl Nov 15, 2024

Choose a reason for hiding this comment

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

I agree, on a small screen (14 in laptop), "tabbed" variant is not an optimal solution.

It looks like I've missed your other reply. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your suggestions. I've diverted corresponding updates into a separate patch that needs more love, in order to get this one merged.

@amotl amotl marked this pull request as ready for review November 14, 2024 08:25
Copy link

@kneth kneth left a comment

Choose a reason for hiding this comment

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

I have only minor suggestions


:::{rubric} Overview
:::
All the CrateDB connectivity options at a glance.
Copy link

Choose a reason for hiding this comment

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

Suggested change
All the CrateDB connectivity options at a glance.
CrateDB connectivity options at a glance.

properties.put("password", "");
properties.put("ssl", false);
Connection conn = DriverManager.getConnection(
"jdbc:crate://localhost:5432/",
Copy link

Choose a reason for hiding this comment

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

please consider to add a comment here about the two different drivers

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Do you agree with that variant?

// You can use both drivers, vanilla pgJDBC, or CrateDB's pgJDBC.
String dburi = "jdbc:crate://localhost:5432/";
String dburi = "jdbc:postgresql://localhost:5432/";
public class Main {
public static void main(String[] args) {
try {
Properties properties = new Properties();
properties.put("user", "crate");
properties.put("password", "");
properties.put("ssl", false);
Connection conn = DriverManager.getConnection(
dburi,
properties
);

@amotl amotl requested a review from kneth November 14, 2024 09:52
@amotl amotl marked this pull request as draft November 14, 2024 11:02
@amotl amotl marked this pull request as ready for review November 15, 2024 18:06
@amotl amotl merged commit b91a1ed into main Nov 15, 2024
4 checks passed
@amotl amotl deleted the amo/feature-connectivity branch November 15, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed cross linking Linking to different locations of the documentation. guidance Matters of layout, shape, and structure. refactoring Changing shape or layout, or moving content around. sanding-180 Sanding rough edges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants