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

Add README badges #624

Merged
merged 12 commits into from
Feb 27, 2024
Merged

Add README badges #624

merged 12 commits into from
Feb 27, 2024

Conversation

garrettmflynn
Copy link
Member

This PR adds badges to the README file for the GUIDE to quickly provide a status check on the repo

@CodyCBakerPhD
Copy link
Collaborator

@garrettmflynn Can you update this to include the new 'external' separation?

Also I'm more used to seeing badges above the logo of a repo; what do you think?

@CodyCBakerPhD
Copy link
Collaborator

Also the video on the README page is now well outdated; do you think we should even include it there?

@garrettmflynn
Copy link
Member Author

Also I'm more used to seeing badges above the logo of a repo; what do you think?

I've usually seen them below the identifier for the repo (i.e. name, logo, etc.). Numpy, Vite, and React (although this is inline) all use this convention.

Though I'm fine with either.

@garrettmflynn
Copy link
Member Author

It seems best to keep the README up to date with the particular branch, and the RTD consistent with the latest release.

How does that sound? In that case, we could move the video to the RTD—or cut it entirely if we think people will not get any benefit from referencing it.

@CodyCBakerPhD
Copy link
Collaborator

It seems best to keep the README up to date with the particular branch, and the RTD consistent with the latest release.

Neither of those are true for the video though, lol

or cut it entirely if we think people will not get any benefit from referencing it.

It's a broader question; it was our main demo back in the day (made for the NWB exec board presentation), but once we have the tutorial and screenshots that will effectively replace that

There are many features present in the video that are still there, but since they are not totally in sync it can be more than a little distracting looking at the video and app side by side

A video is in general also more work to keep up to date, but maybe we could plan a new one for the first official release? Just as a really quick advertisement of appearance/capability

So I would say remove for now and bring it up in next group GUIDE meeting

@garrettmflynn
Copy link
Member Author

Yeah, I think a completely new video per major release—as well as short standalone videos for any important feature additions in minor releases—makes sense.

@CodyCBakerPhD
Copy link
Collaborator

I've usually seen them below the identifier for the repo (i.e. name, logo, etc.). Numpy, Vite, and React (although this is inline) all use this convention.

Huh, right you are!

I like the Vite one especially; can you center align the badges like that? Also maybe add code style ones as a separate line (black and prettier?) - maybe NWB slack/contact page too? Badges are neat

@garrettmflynn
Copy link
Member Author

Can do :)

@garrettmflynn
Copy link
Member Author

Do you have an example for Black and Prettier? Not sure where to link to.

@CodyCBakerPhD
Copy link
Collaborator

<a href="https://github.com/psf/black"><img alt="Code style: black" src="https://img.shields.io/badge/code%20style-black-000000.svg"></a>

as seen on

https://github.com/psf/black/blob/main/README.md

I don't actually know if prettier has one but I would assume it's not difficult to do

@garrettmflynn
Copy link
Member Author

Updated!

@CodyCBakerPhD
Copy link
Collaborator

Can you update this to include the new 'external' separation?

That is, separate badges for the 'external' (which will show as failing right now) and the other ones (which should be passing); also tests on the build+distributable workflow

Also could you separate the 'build and deploy' as a different line?

@garrettmflynn
Copy link
Member Author

Done! Thanks for bumping

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@CodyCBakerPhD CodyCBakerPhD enabled auto-merge (squash) February 27, 2024 20:20
@CodyCBakerPhD
Copy link
Collaborator

Hmm

Rather strange, but this PR gives a couple missing components on the UI tests: https://www.chromatic.com/build?appId=640a4d93e34604f275368983&number=835

But those changes didn't show up on previous code-based merges earlier today

@garrettmflynn
Copy link
Member Author

Hmm weird. These are quite old.

@CodyCBakerPhD CodyCBakerPhD merged commit 472d870 into main Feb 27, 2024
12 of 16 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the readme-badges branch February 27, 2024 20:38
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.

2 participants