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

Enhance Developer guide and experience #7218

Closed
wants to merge 9 commits into from

Conversation

MarcSkovMadsen
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen commented Sep 1, 2024

Closes #7217 and #7220

Copy link

codecov bot commented Sep 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.25%. Comparing base (82dcddd) to head (10d72e3).
Report is 56 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7218   +/-   ##
=======================================
  Coverage   82.25%   82.25%           
=======================================
  Files         331      331           
  Lines       49460    49461    +1     
=======================================
+ Hits        40682    40684    +2     
+ Misses       8778     8777    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MarcSkovMadsen MarcSkovMadsen marked this pull request as ready for review September 1, 2024 03:46

To be able to run cells interactively you need `pyodide` server, this can be ran with:
```bash
pixi run _docs-build-api-reference
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've never understood the docs build flow before. But now I think I do. Making the sub tasks more visible and giving them more readable names will help a me lot. I believe it will help others too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For example all the broken links we have. Now I think I now better how to fix it because I understand how the nbsite build will see the examples.

Copy link
Member

Choose a reason for hiding this comment

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

Making the sub tasks more visible ... I believe it will help others too.

I explicitly made them non-visible. The biggest problem is that one of the tasks takes 95 % or more of the time; this should be improved at a fundamental level with how nbsite builds the documentation.

Two things should be considered when working with development tools: using them and debugging/fixing problems with them.

Most contributors should "just" use the development tool, which should be as easy as possible. This is why it is one command and hides the subcommands. Most users should not need to remember (or even know) five subcommands and which order they should be run in.

You are having problems debugging or fixing a problem caused by the development tool you are using, which is great! But it is, or should be, limited to a handful of people.

This suggestion falls into the latter category and could, in my opinion, confuse new developers more than help them.

... and giving them more readable names will help a me lot.

No problem with better names.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But there is no need to spend time on creating api Gallery or Pyodide output if I'm working on some markdown document.

Copy link
Member

Choose a reason for hiding this comment

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

Agree with that, that said only certain subtasks should be pulled out, I'd say currently this makes sense:

  • docs-build-sphinx
  • docs-build-pyodide-gallery

and then I'd ideally to have a docs-preview task so you can preview individual docs pages without building the entire sphinx site.

@@ -99,6 +126,8 @@ pixi run -e test-ui install

:::

You can find the names of the environments in the **pixi.toml** file.
Copy link
Member

Choose a reason for hiding this comment

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

Seems better to recommend running pixi info and pixi task list.

@cdeil cdeil mentioned this pull request Sep 1, 2024
1. Go to [github.com/holoviz/panel](https://github.com/holoviz/panel)
2. [Fork the repository](https://docs.github.com/en/get-started/quickstart/contributing-to-projects#forking-a-repository)
3. Run in your terminal: `git clone https://github.com/<Your Username Here>/panel`

:::

:::{tab-item} Core Contributor
Copy link
Member

Choose a reason for hiding this comment

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

I feel that if you are a core contributor, you have shown your worth and do not need guidance on how to clone a repo.

This page is meant for new contributors, not experienced ones.

Copy link
Member

Choose a reason for hiding this comment

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

Agree.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that you know how to do it.

But as any contributor you go here to get installed. Its multiple steps and they change over time.

It is convenient you can Copy into terminal instead of spdnding additional time compsing commands.

Now navigate to the root of the `panel` project:

```bash
cd panel
Copy link
Member

Choose a reason for hiding this comment

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

If you can't cd into a directory, you are likely unable to contribute 🙃

Copy link
Collaborator Author

@MarcSkovMadsen MarcSkovMadsen Sep 1, 2024

Choose a reason for hiding this comment

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

But for example if you are leading a sprint at a conference some people with minimal technical skills will join to not only contribute, but to learn.

I would argue we should make the developer guide as beginner friendly and explicit as possible.


To be able to run cells interactively you need `pyodide` server, this can be ran with:
```bash
pixi run _docs-build-api-reference
Copy link
Member

Choose a reason for hiding this comment

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

Making the sub tasks more visible ... I believe it will help others too.

I explicitly made them non-visible. The biggest problem is that one of the tasks takes 95 % or more of the time; this should be improved at a fundamental level with how nbsite builds the documentation.

Two things should be considered when working with development tools: using them and debugging/fixing problems with them.

Most contributors should "just" use the development tool, which should be as easy as possible. This is why it is one command and hides the subcommands. Most users should not need to remember (or even know) five subcommands and which order they should be run in.

You are having problems debugging or fixing a problem caused by the development tool you are using, which is great! But it is, or should be, limited to a handful of people.

This suggestion falls into the latter category and could, in my opinion, confuse new developers more than help them.

... and giving them more readable names will help a me lot.

No problem with better names.

pixi.toml Outdated Show resolved Hide resolved
@@ -209,6 +247,7 @@ You will likely want to check out the

- [Dev version of Panel Site](https://holoviz-dev.github.io/panel)
- Use this to explore new, not yet released features and docs
- See which branch is currently deployed [here](https://github.com/holoviz/panel/actions/workflows/docs.yaml)
Copy link
Member

Choose a reason for hiding this comment

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

You need to know what you are looking for to see what branch is deployed as the docs can be built in a dry run option.

pixi.toml Outdated Show resolved Hide resolved
doc/developer_guide/index.md Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Sep 15, 2024

I can see we don't agree and thus it does not make sense to try to push for what I believe are improvements.

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.

Align names of pixi tasks
3 participants