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

Lecture2 branch #19

Merged
merged 4 commits into from
Sep 5, 2024
Merged

Lecture2 branch #19

merged 4 commits into from
Sep 5, 2024

Conversation

Emma-airi
Copy link
Collaborator

Corrections made on Lecture2, ready for final review.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Joosty
Copy link
Collaborator

Joosty commented Sep 3, 2024

Just a few things to correct:

  • 1. Contents not working: See below an image of the last two contents links not functioning. image

  • 2. Time is not a vector Time has been made bold as if it were a vector when it is a scalar quantity. image

  • 3. Rogue $ signs There are cases of rogue dollar signs. This can occur if you accidentally use double dollar signs without leaving a gap so it becomes a linear equation.
    image

  • 4. Random 1+2
    image

  • 5. Inconsistent symbols Please go through and check wether each symbol should be bold or non bold and also please ensure consistency in the symbols used. eg here with the 'v'.
    image

Hope this helped :)
Joost

@angadhn
Copy link
Owner

angadhn commented Sep 3, 2024

Bloody hell, @Joosty. Beat me to the punch!

@angadhn angadhn requested a review from Joosty September 3, 2024 16:54
@Joosty
Copy link
Collaborator

Joosty commented Sep 3, 2024

@angadhn I may have missed something so I'm sure there is something left for you to comment on lol

@Joosty
Copy link
Collaborator

Joosty commented Sep 4, 2024

Okay so after building there are a few issues me or you must've missed last time:

  • 1. '/bf' in contents not rendering
    image

  • 2. '' Symbols appearing around time symbols They are also still bold (they are not vectors and should not be). This appears across the whole text.
    image

I will leave more comments (as i have seen a few more issues) in around an hour or two (i have an induction session for ESA at 1).

I would suggest if you have not been, building the book locally to test out the rendering before committing/pushing.

Cheers,
Joost

@Emma-airi
Copy link
Collaborator Author

Okay so after building there are a few issues me or you must've missed last time:

  • 1. '/bf' in contents not rendering
    image
  • 2. '' Symbols appearing around time symbols They are also still bold (they are not vectors and should not be). This appears across the whole text.
    image

I will leave more comments (as i have seen a few more issues) in around an hour or two (i have an induction session for ESA at 1).

I would suggest if you have not been, building the book locally to test out the rendering before committing/pushing.

Cheers, Joost

Yes, i have not made time to build the book yet. Ill do it now before i continue with the other files i will be commiting later. Thank you Joost!

@Joosty
Copy link
Collaborator

Joosty commented Sep 4, 2024

Okay so after building there are a few issues me or you must've missed last time:

  • 1. '/bf' in contents not rendering
    image
  • 2. '' Symbols appearing around time symbols They are also still bold (they are not vectors and should not be). This appears across the whole text.
    image

I will leave more comments (as i have seen a few more issues) in around an hour or two (i have an induction session for ESA at 1).
I would suggest if you have not been, building the book locally to test out the rendering before committing/pushing.
Cheers, Joost

Yes, i have not made time to build the book yet. Ill do it now before i continue with the other files i will be commiting later. Thank you Joost!

Will get on with finding any more issues now :)

@Joosty
Copy link
Collaborator

Joosty commented Sep 4, 2024

@Emma-airi

  • 3. Rogue $ signs
    image
    image
    image

  • 4. '*' symbol
    image

  • 5. Bold vs non-bold symbols Across the equations in this file there is an issue with things that are not vectors being highlighted in bold and the same vice versa. See below for some examples - I am skimming here so please check to make sure I am correct on some of these as you have the original notes. image
    image
    image
    image
    image
    image

  • 6. Unnecessary/ confusing equation labels
    image

I will keep an eye out for anything I missed, any questions please ask :)

@angadhn
Copy link
Owner

angadhn commented Sep 5, 2024

@Joosty the issues you raised look resolved. If you think all looks good, we can merge this. You can do the honours :-)

@Joosty
Copy link
Collaborator

Joosty commented Sep 5, 2024

Will review and then merge in 30 mins :)

@Joosty
Copy link
Collaborator

Joosty commented Sep 5, 2024

@Emma-airi @angadhn Looking at it all seems good, the only thing I was unsure about was a few symbols being bold or non-bold which seems to be resolved.

EDIT
I think it needs a GitHub review approved from the code owner @angadhn before I can merge.
image

@angadhn angadhn merged commit c0cbe96 into angadhn:main Sep 5, 2024
1 check passed
@angadhn
Copy link
Owner

angadhn commented Sep 5, 2024

@Emma-airi @angadhn Looking at it all seems good, the only thing I was unsure about was a few symbols being bold or non-bold which seems to be resolved.

EDIT I think it needs a GitHub review approved from the code owner @angadhn before I can merge. image

Hmmm. Okay. Thanks!

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.

3 participants