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

PR for updating docs theme #877

Merged
merged 8 commits into from
Feb 26, 2024
Merged

PR for updating docs theme #877

merged 8 commits into from
Feb 26, 2024

Conversation

nucleosynthesis
Copy link
Contributor

This PR modifies the theme to be more like those used in other CMSSW codes (eg the CAT doc pages). No need to merge until we're ready to make that change.

@nucleosynthesis nucleosynthesis added the documentation Updates for the documentation label Feb 6, 2024
@nucleosynthesis
Copy link
Contributor Author

many of the changes are already merged into the main branch - only changes to mkdocs and index are relevant

@kcormi
Copy link
Collaborator

kcormi commented Feb 22, 2024

Hi Nick,

I tried out this theme and the build, it looks good to me, I think its a nice a nice improvement over the current one. And all the various boxes/features etc seem to be working.

The only small thing I noticed, is I think in the current PR it is using an old version of the logo? The one on the pages has the fairly saturated colours, where I thought we were switching to the less saturated version.

NB: I didn't check to make sure the content is unchanged, I have only been checking for style so far.

@nucleosynthesis
Copy link
Contributor Author

Hi @kcormi - thanks, yeah the logo is a little outdated, currently getting the latest and greatest one from Clemens to add in.

Copy link
Collaborator

@kcormi kcormi left a comment

Choose a reason for hiding this comment

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

I just rebased this PR on top of the main, to see the changes more clearly.

The only conflict was in the physicsmodel.md, which is now consistent although I don't know if we prefer to use $\vec{\mu}$ instead of $\vec{\theta}$ since I think the first one is consistent with the paper. This could also be updated in another PR, where we might want to check the notation and make it as consistent as we can though.

@nucleosynthesis, please just check that nothing got missed or changed in an unintended way.

@nucleosynthesis
Copy link
Contributor Author

Looks good to me - I think probably going with the \mu in the physics model is better since we use that in the paper (and in the rest of the docs I already tried to go through and update to use the paper notation where I could)

@kcormi
Copy link
Collaborator

kcormi commented Feb 26, 2024

Looks good to me - I think probably going with the \mu in the physics model is better since we use that in the paper (and in the rest of the docs I already tried to go through and update to use the paper notation where I could)

Thanks, I've updated it to use $\vec{\mu}$ and $y$.

From my side the PR looks ready.

@nucleosynthesis
Copy link
Contributor Author

I'd be happy to have this one merged, since others are making edits to the docs (from the CAT hackathon), we should probably get this one in so we don't have conflicts later?

@anigamova anigamova merged commit 75ce783 into main Feb 26, 2024
6 checks passed
@nucleosynthesis nucleosynthesis deleted the nckw-theme-change branch March 26, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Updates for the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants