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

Updated Logogistiks PR #2

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Updated Logogistiks PR #2

wants to merge 3 commits into from

Conversation

aleph23
Copy link
Owner

@aleph23 aleph23 commented Jul 25, 2024

May already be in my fork - diff PR to confirm.

Summary by Sourcery

This pull request introduces a comprehensive 'Camera' section to the index.html, enhancing the document with detailed explanations and examples of different camera shot sizes, framing techniques, focus methods, and angles.

  • New Features:
    • Added a new 'Camera' section to the index.html, providing detailed information on various camera shot sizes, framing techniques, focus methods, and angles.

I added a section for various camera styles like angles, framing, shot focus and shot size
Copy link

sourcery-ai bot commented Jul 25, 2024

Reviewer's Guide by Sourcery

This pull request enhances the existing HTML document by adding a new 'Camera' section. This section provides an in-depth guide on various camera techniques, including shot sizes, framing, focus, and angles. The changes include new menu navigation, detailed descriptions, and illustrative images to help users understand different camera techniques. Additionally, external references are provided for further reading.

File-Level Changes

Files Changes
index.html Enhanced the HTML document by adding a comprehensive new section on camera techniques, including shot sizes, framing, focus, and angles, along with illustrative images and external references.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @aleph23 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider breaking this large PR into smaller, more focused changes. This would make it easier to review and manage.
  • The HTML structure is quite repetitive. Consider using templates or a more modular approach to reduce duplication and improve maintainability.
  • Ensure that all referenced images are present in the repository or will be added in a separate PR. Also, consider adding appropriate CSS and JavaScript to enhance the presentation and interaction of this new content.
Here's what I looked at during the review
  • 🟡 General issues: 3 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Comment on lines +494 to +501
<h4>Medium Wide Shot (MWS)</h4>
<p>A medium long shot (aka medium long shot) frames the subject from roughly the knees up. It splits the
difference
between a full shot and a medium shot. Here's an example of the medium wide shot size from the movie The
Usual Suspects:
</p>
<img width="1600" src="img/camera/shot_size/MWS.webp">
<p>You can always frame camera shots from any angle as well, so don't be afraid to think about medium long shots
Copy link

Choose a reason for hiding this comment

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

issue: Inconsistent terminology for Medium Wide Shot.

The term 'Medium Wide Shot' is abbreviated as 'MWS' but is also referred to as 'medium long shot' in the description. Consider standardizing the terminology to avoid confusion.

<p>The medium close-up camera shot size keeps the characters eerily distant even during their face-to-face
conversation.
</p>
<h4>Close Up (CU)</h4>
Copy link

Choose a reason for hiding this comment

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

issue: Duplicate content in Close Up section.

The description for the Close Up shot is repeated twice. Consider removing the duplicate content to improve readability.

cityscapes.
</p>
<img width="1600" src="img/camera/angles/Aerial_Shot.webp">
<h3>All Content is taken from this article: <a href="https://www.studiobinder.com/blog/ultimate-guide-to-camera-shots/" target="_blank">Ultimate guide to camera shots</a></h3>
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider rephrasing the attribution statement.

The attribution statement 'All Content is taken from this article' could be rephrased to 'All content is sourced from this article' for better readability.

Suggested change
<h3>All Content is taken from this article: <a href="https://www.studiobinder.com/blog/ultimate-guide-to-camera-shots/" target="_blank">Ultimate guide to camera shots</a></h3>
<h3>All content is sourced from this article: <a href="https://www.studiobinder.com/blog/ultimate-guide-to-camera-shots/" target="_blank">Ultimate guide to camera shots</a></h3>

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.

None yet

2 participants