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

Review compositor tutorial #3606

Merged
merged 10 commits into from
Oct 9, 2024
Merged

Conversation

yhontyk
Copy link
Contributor

@yhontyk yhontyk commented Sep 23, 2024

Another docs review. I've provided a bit more context to the code creation part (at least as well as I myself could understand it).
Do you think the build definition of Cmake requires more explanations? Or will it be a self-evident thing for a regular C++ developer?

Jira: https://warthogs.atlassian.net/browse/DOCPR-852

@yhontyk yhontyk requested a review from a team as a code owner September 23, 2024 15:21
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Do you think the build definition of Cmake requires more explanations? Or will it be a self-evident thing for a regular C++ developer?

CMake is the de facto standard for C++ development. It is also fairly self explanatory: developers accustomed to different tooling will be able to extract the details they need.

## Running a Mir composer
You can run a composer nested in an X or Wayland session, or from a virtual terminal, just like the demo applications in [Getting started with Mir](learn-what-mir-can-do.md).

For example, to run inside an existing Wayland session:
```sh
WAYLAND_DISPLAY=wayland-99 ./build/demo-mir-compositor
Copy link
Contributor

Choose a reason for hiding this comment

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

To the uninitiated like me, wayland-99 seems like a magic number. Maybe it can get a note or a comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually, Wayland compositors use wayland-0 (or wayland-1, wayland-2, ... if you're running multiple compositors). This is used as the name of the compositor's socket which two compositors cannot share (as far as I know). So wayland-99 is just a convention we use in the team to not have to worry about that.

Co-authored-by: Artem Konev <[email protected]>
Co-authored-by: Alan Griffiths <[email protected]>
## Installing dependencies

The example program requires
* `libmiral`- a library for Mir abstraction layer
Copy link
Contributor

Choose a reason for hiding this comment

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

Some suggestions: (looks like I misclicked something when starting the review :))

doc/sphinx/tutorial/write-your-first-wayland-compositor.md Outdated Show resolved Hide resolved
doc/sphinx/tutorial/write-your-first-wayland-compositor.md Outdated Show resolved Hide resolved
doc/sphinx/tutorial/write-your-first-wayland-compositor.md Outdated Show resolved Hide resolved
doc/sphinx/tutorial/write-your-first-wayland-compositor.md Outdated Show resolved Hide resolved
doc/sphinx/tutorial/write-your-first-wayland-compositor.md Outdated Show resolved Hide resolved
doc/sphinx/tutorial/write-your-first-wayland-compositor.md Outdated Show resolved Hide resolved
AlanGriffiths and others added 2 commits October 8, 2024 12:06
Co-authored-by: tarek-y-ismail <[email protected]>
Co-authored-by: tarek-y-ismail <[email protected]>
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Another typo

doc/sphinx/tutorial/write-your-first-wayland-compositor.md Outdated Show resolved Hide resolved
Co-authored-by: Alan Griffiths <[email protected]>
@tarek-y-ismail tarek-y-ismail added this pull request to the merge queue Oct 9, 2024
Merged via the queue into canonical:main with commit 8f9271b Oct 9, 2024
24 checks passed
@tarek-y-ismail
Copy link
Contributor

@akcano & @yhontyk: Probably should've said this earlier, but thanks for your contributions!

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