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

POC Controller api docu #562

Draft
wants to merge 2 commits into
base: 2.4
Choose a base branch
from

Conversation

JoergAtGithub
Copy link
Member

@JoergAtGithub JoergAtGithub commented May 13, 2023

@JoergAtGithub JoergAtGithub changed the base branch from 2.3 to 2.4 May 13, 2023 22:38
@daschuer
Copy link
Member

I am unsure about this. At least this should not be part of the normal user manual. Because it is useless info for the majority of users. This will unnecessarily increase the PDF manual size and increase the amount of strings for translation.

We can think of an online only version. But I personally would not use it. Instead I would use the source files directly which also contain the same info.

What is the use case for these pages?

@Swiftb0y
Copy link
Member

I am unsure about this. At least this should not be part of the normal user manual. Because it is useless info for the majority of users. This will unnecessarily increase the PDF manual size and increase the amount of strings for translation.

The same applies to the Mixxx Controls in the appendix. We decided to put it in the manual so we can version it. IMO the same argument applies to other API docs. Though I certainly agree that there is little value in translating these docs (same for the Mixxx Controls).

We can think of an online only version. But I personally would not use it. Instead I would use the source files directly which also contain the same info.

What is the use case for these pages?

IMO the source files can be a little verbose, especially for beginners, having a more readable documentation would help those a lot.

@daschuer
Copy link
Member

daschuer commented May 14, 2023

The same applies to the Mixxx Controls in the appendix. We decided to put it in the manual so we can version it. IMO the same argument applies to other API docs.

Yes indeed. But this does not mean that this decision was good.

There is major difference betten the control description and the controller API description: You will get in touch with the controls as a normal user from Mixxx itself via the Mapping GUI. The behavior definition of the controls is spreaded over the whole Mixxx source code. The is no self documentation in the Mixxx code base itself.

The current state of this API documentation quite basically. And probably disappointing. In this state it does not help much compared to the maninanance burden and when reading the source file directly. For me it is pointless to describe a simple function in prose, when the source code already tells the truth. This is especially true if you use an IDE that is able to present the API documentaiin as a tooltip/intelisense.

But if you like to start a comprehend developer/mapper guid it is helpful for sure, but also a big project.

@Swiftb0y
Copy link
Member

The current state of this API documentation quite basically. And probably disappointing. In this state it does not help much compared to the maninanance burden and when reading the source file directly. For me it is pointless to describe a simple function in prose, when the source code already tells the truth. This is especially true if you use an IDE that is able to present the API documentaiin as a tooltip/intelisense.

I disagree. intellisense does not replace documentation, even basic. Intellisense is great reminder of the details while you're actually coding, but it does not give you an overview of the API surface. The Documentation here, while rudimentary is still quite comprehensive. Unfortunately, its not technically possible with a reasonable amount of work to automatically generate docs for the controller APIs (which have been written in C++) to generate automatically, so typescript declarations are the next closest thing. We already have the controller mapping pages in the wiki, we could probably move their content into the typescript declarations and generate the controller mapping API from that. The result is no more redundant than what we currently have.

But if you like to start a comprehend developer/mapper guid it is helpful for sure, but also a big project.

I agree that it would be reasonable to have the controller mapping doc separate, but for now adding it to the appendix in the manual is fine. I'd be okay with blacklisting these autogenerated pages from translation and pdf build if thats possible.

@daschuer
Copy link
Member

We had a similar discussion about the doxygen Doku of the Mixxx source code. And we came to the conclusion that probably no one is reading the generated HTML files once dived into the code. The same thing applies here. Apart of the formatting, there is not more information in https://github.com/mixxxdj/mixxx/blob/2.4/res/controllers/color-mapper-api.d.ts compared to https://github.com/mixxxdj/manual/blob/581e6b9ddcb3e7643a1c93864abceac33f617fc2/source/chapters/controller_api/classes/color_mapper_api.ColorMapper.md

At least that does not rectify to blow up the PDF document that is shipped with Mixxx on every user installation, targeting the DJ-ing user. The API documentation will also clutter the search results with useless hits for the normal user. On the Other I assume hand the mapping author will use "find in files" with his IDE and not use the manual at all.

I think it is a very good idea to also move also the information from the wiki to the source code as far as possible.

@Swiftb0y
Copy link
Member

Thats fair. Whats your opinion @JoergAtGithub?

@Holzhaus
Copy link
Member

Holzhaus commented May 15, 2023

IMO the PDF document has bad UX anyway. We should strive for an HTML-based manual. And if we do that, we can easily add the controller documentation.

The alternative would be a separate "mapping/skinning manual", but I think this would be confusing and lead to a lot of redundant information.

@Swiftb0y
Copy link
Member

Not sure about the redundancy, I don't think there is much overlap between usage documentation and controller mapping documentation, we might want to move the rest of the wiki to that new doc as well, the wiki has lost most of its advantages anyways after we had to lock it down due to malware spam.

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.

4 participants