Skip to content

Conversation

@JoschD
Copy link
Member

@JoschD JoschD commented Mar 31, 2025

Added my presentation from today. What do you think?

@JoschD JoschD added Type: Documentation Improvements, updates and fixes to the documentation. Estimate: Easy Good first issue for newcomers. Straightforward fixes. Priority: Low Work on this if you have some spare time. labels Mar 31, 2025
@JoschD JoschD self-assigned this Mar 31, 2025
@JoschD JoschD requested review from fsoubelet and jgray-19 March 31, 2025 09:52
jgray-19
jgray-19 previously approved these changes Mar 31, 2025
Copy link
Contributor

@jgray-19 jgray-19 left a comment

Choose a reason for hiding this comment

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

I like the addition. It's good to know also what is done automatically to your data documented. So, looks good to me.

Copy link
Member

@fsoubelet fsoubelet left a comment

Choose a reason for hiding this comment

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

Few typos here and there. Two things though:

  • I'm not sure we need the images in the website. I liked them in the slides but I'd like to keep the repo and builds small.
  • I don't know if we should be introducing some new css for pages. Might be a lot at some point.

@JoschD
Copy link
Member Author

JoschD commented Mar 31, 2025

To your points: I agree with them in the sense, that I also thought about the same things, but decided to go this way because:

a) The images are not needed, but I liked them and also reduced the larger ones in size and they are all <600KB. I think that is acceptable.

b) the css colors are nice to have and the additional styles are named very specifically, so they should not interfer with anything else. I think they increase the points made in the text by sorting them into different analysis steps/topics.

@JoschD JoschD requested a review from fsoubelet March 31, 2025 12:28
Copy link
Member

@fsoubelet fsoubelet left a comment

Choose a reason for hiding this comment

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

No strong opinions on these points from me, all good to merge. Thanks

@JoschD JoschD merged commit a8b8b5d into master Mar 31, 2025
2 checks passed
@JoschD JoschD deleted the bad_bpms_method branch March 31, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Estimate: Easy Good first issue for newcomers. Straightforward fixes. Priority: Low Work on this if you have some spare time. Type: Documentation Improvements, updates and fixes to the documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants