Skip to content

beman.exemplar: change library status to under development (exemplar is a template library)#89

Merged
neatudarius merged 1 commit intobemanproject:mainfrom
neatudarius:LIBRARY_STATUS_UPDATE
Jan 19, 2025
Merged

beman.exemplar: change library status to under development (exemplar is a template library)#89
neatudarius merged 1 commit intobemanproject:mainfrom
neatudarius:LIBRARY_STATUS_UPDATE

Conversation

@neatudarius
Copy link
Copy Markdown
Member

@neatudarius neatudarius commented Dec 20, 2024

Issues: bemanproject/beman#77

Expected README.md:

image

@neatudarius neatudarius marked this pull request as ready for review December 20, 2024 22:47
@neatudarius neatudarius changed the title Change library status to under development (exemplar is a template library) beman.exemplar: change library status to under development (exemplar is a template library) Dec 20, 2024
@RaduNichita RaduNichita self-requested a review December 20, 2024 22:55
@neatudarius neatudarius force-pushed the LIBRARY_STATUS_UPDATE branch from 4d2db3c to b168053 Compare December 20, 2024 23:20
@neatudarius
Copy link
Copy Markdown
Member Author

@wusatosi , what should I do here?
image
I didn't change these lines. And I want to keep them! (collapsable menus in docs).

@wusatosi
Copy link
Copy Markdown
Member

@wusatosi , what should I do here?
image
I didn't change these lines. And I want to keep them! (collapsable menus in docs).

Hum, weird, inline HTML check should have been disabled. I will look into this.

@neatudarius
Copy link
Copy Markdown
Member Author

@wusatosi , what should I do here?
image
I didn't change these lines. And I want to keep them! (collapsable menus in docs).

Hum, weird, inline HTML check should have been disabled. I will look into this.

OK. Then I will proceed with the PR, as this behaviour is not from changed lines.

Copy link
Copy Markdown
Contributor

@RaduNichita RaduNichita left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread README.md Outdated
Comment thread README.md Outdated
Copy link
Copy Markdown
Member

@wusatosi wusatosi left a comment

Choose a reason for hiding this comment

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

OK. Then I will proceed with the PR, as this behaviour is not from changed lines.

No. lint break is caused by changed lines.

Avoid fighting with markdownlint with ignore statements, ignore should be the last resort.

Put faith into the linting infrastructure, let's not have our code littered with lint-ignores.

@neatudarius
Copy link
Copy Markdown
Member Author

OK. Then I will proceed with the PR, as this behaviour is not from changed lines.

No. lint break is caused by changed lines.

Avoid fighting with markdownlint with ignore statements, ignore should be the last resort.

Put faith into the linting infrastructure, let's not have our code littered with lint-ignores.

I agree with any solution which can work right now. So what should I do if the CI complains the line is too long? And I actually want to insert a text with hyperlink? (The text is short, the hyperlink is long).

Currently, the infrastructure seems to not consider this case. If this is true, it should be solved in other issue/PR.

@JeffGarland JeffGarland self-assigned this Dec 23, 2024
Copy link
Copy Markdown
Member

@JeffGarland JeffGarland left a comment

Choose a reason for hiding this comment

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

Overall this look good.

@neatudarius neatudarius force-pushed the LIBRARY_STATUS_UPDATE branch 2 times, most recently from 0a443e5 to 508df8c Compare December 24, 2024 18:43
@wusatosi
Copy link
Copy Markdown
Member

OK. Then I will proceed with the PR, as this behaviour is not from changed lines.

No. lint break is caused by changed lines.
Avoid fighting with markdownlint with ignore statements, ignore should be the last resort.
Put faith into the linting infrastructure, let's not have our code littered with lint-ignores.

I agree with any solution which can work right now. So what should I do if the CI complains the line is too long? And I actually want to insert a text with hyperlink? (The text is short, the hyperlink is long).

Currently, the infrastructure seems to not consider this case. If this is true, it should be solved in other issue/PR.

It looks like markdown lint ignores length overflow due to url via []().

@neatudarius
Copy link
Copy Markdown
Member Author

It looks like markdown lint ignores length overflow due to url via []().

@wusatosi , would you agree with current PR state? Check rendered version.

@neatudarius
Copy link
Copy Markdown
Member Author

@wusatosi , can you take a look?

@wusatosi
Copy link
Copy Markdown
Member

wusatosi commented Jan 9, 2025

It looks like markdown lint ignores length overflow due to url via []().

@wusatosi , would you agree with current PR state? Check rendered version.

Hey sorry I missed this PR.
Is it possible to just use a generic table to format this?

Also, since the image is not in the table, why not just use the markdown facility for image embedding over <img>?

image

Edit: agh, I see, it takes the entire page.

@wusatosi
Copy link
Copy Markdown
Member

wusatosi commented Jan 9, 2025

Also, I think you should reference the direct asset link.

Instead of the github link . The link you provided does not get displayed correct everywhere.

Or you should add raw=true to the end of the github link. See: this stackoverflow post.

e.g. in visual studio/ our codespace environment
image

@wusatosi
Copy link
Copy Markdown
Member

wusatosi commented Jan 9, 2025

I do want to point out that "under development and not yet ready for production use." does not have good visibility on a 100% zoom view.

image

And is invisible on your own screenshot.

image

Edit: there is readability issue on mobile as well.
Screenshot_20250109-105754~2.png

@wusatosi
Copy link
Copy Markdown
Member

wusatosi commented Jan 9, 2025

Why not, instead of putting this cute logo on the front, we just put a badge here. e.g. Beman lifetime status: Under development

# beman.exemplar: A Beman Library Exemplar

![Beman lifetime status: Under development](https://img.shields.io/badge/Beman%20lifetime-under%20dvelopment-blue)
![Continuous Integration Tests](https://github.com/bemanproject/exemplar/actions/workflows/ci_tests.yml/badge.svg)
![Lint Check (pre-commit)](https://github.com/bemanproject/exemplar/actions/workflows/pre-commit.yml/badge.svg)

`beman.exemplar` is a minimal C++ library conforming to [The Beman Standard](https://github.com/bemanproject/beman/blob/main/docs/BEMAN_STANDARD.md).
This can be used as a template for those intending to write Beman libraries.
It may also find use as a minimal and modern  C++ project structure.

This is a lot cleaner, there's no readability issue and weird formatting, its dense and easy to glance over.
Its all native markdown, should work across all markdown renders/ linters.

See: my alternative with badge

image

We could still put the cute logo in a later section to allow branding and a more detailed write out.

@wusatosi
Copy link
Copy Markdown
Member

wusatosi commented Jan 9, 2025

If we still want the logo, would this be a better altnerative? See this as an example

image

Note: We could still add Status below the implementation back. As this would violate [README.LIBRARY_STATUS]

Copy link
Copy Markdown
Member

@wusatosi wusatosi left a comment

Choose a reason for hiding this comment

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

Concern regarding the readability.

We can proceed without the icon for now and move the discussion to discourse.

https://discourse.bemanproject.org/t/readability-concern-with-the-new-beman-lifetime-notation/301

Comment thread README.md Outdated
@neatudarius neatudarius force-pushed the LIBRARY_STATUS_UPDATE branch from 508df8c to 70c3086 Compare January 16, 2025 18:32
@neatudarius
Copy link
Copy Markdown
Member Author

neatudarius commented Jan 16, 2025

Concern regarding the readability.

We can proceed without the icon for now and move the discussion to discourse.

https://discourse.bemanproject.org/t/readability-concern-with-the-new-beman-lifetime-notation/301

image

@wusatosi , check latest status of this PR.

I proposed to put this 4 SVGs in the beman docs: https://github.com/bemanproject/beman/pull/84/files.

@neatudarius neatudarius force-pushed the LIBRARY_STATUS_UPDATE branch from 70c3086 to e801293 Compare January 16, 2025 18:56
@neatudarius neatudarius force-pushed the LIBRARY_STATUS_UPDATE branch 2 times, most recently from 3c5f23a to 2963198 Compare January 18, 2025 07:43
@wusatosi
Copy link
Copy Markdown
Member

Concern regarding the readability.
We can proceed without the icon for now and move the discussion to discourse.
https://discourse.bemanproject.org/t/readability-concern-with-the-new-beman-lifetime-notation/301

image @wusatosi , check latest status of this PR.

I proposed to put this 4 SVGs in the beman docs: https://github.com/bemanproject/beman/pull/84/files.

This is fantastic, thank you for all these work.

@neatudarius neatudarius merged commit a5baebc into bemanproject:main Jan 19, 2025
@camio camio mentioned this pull request Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants