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

Branch convert to pdf #286

Merged

Conversation

wasjoe1
Copy link
Collaborator

@wasjoe1 wasjoe1 commented Nov 13, 2023

Let's format the documentations better for PDF formats!

Please do not merge this with the master branch, this PR is for the purpose of anyone wanting to make changes to our PDF format

@wasjoe1 wasjoe1 added the documentation Improvements or additions to documentation label Nov 13, 2023
@wasjoe1 wasjoe1 added this to the v1.4 milestone Nov 13, 2023
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ae09f41) 90.85% compared to head (512b4ca) 90.85%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #286   +/-   ##
=========================================
  Coverage     90.85%   90.85%           
  Complexity      832      832           
=========================================
  Files            97       97           
  Lines          2188     2188           
  Branches        305      305           
=========================================
  Hits           1988     1988           
  Misses          129      129           
  Partials         71       71           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@samuelim01 samuelim01 left a comment

Choose a reason for hiding this comment

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

Roughly looks good to me! Just a few comments:

  1. According to the CS2103T website, the UG and DG should match the pdf. As such, I think this PR should be merged. Visually, I see little issue with merging. Perhaps we can make the changes such as the Table of Contents on a branch for CS2101.
  2. I personally feel some of the DG diagrams have been shrunk too much and are now too small.
  3. I removed a irrelevant line from the glossary (Private contact detail). This should be removed from the repo too.

Choose a reason for hiding this comment

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

For the Appendix, the acceptable values for email are split over two pages. Would it be more natural to move the whole section to a new page? The gap would be rather wide though.

Before:
image

After:
image

Choose a reason for hiding this comment

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

The above section between 5.5.2. and 5.6 has a Back to top by itself.

image

@p-xp p-xp removed this from the v1.4 milestone Nov 14, 2023
Copy link

@samuelim01 samuelim01 left a comment

Choose a reason for hiding this comment

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

LGTM! 😄

@samuelim01 samuelim01 merged commit 9ffe25e into AY2324S1-CS2103T-F11-2:master Nov 14, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants