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

Feature: Add link to lesson commit history #4570

Merged

Conversation

MaoShizhong
Copy link
Contributor

@MaoShizhong MaoShizhong commented Jun 17, 2024

Because

It would be good to see when a lesson was last updated and go directly to the lesson's commit history to see the changelog, directly from the lesson page itself. This should help alleviate some confusion when something in the lesson changes when someone is midway through - they'll now be able to see that it was changed and find out what the change was, without having to go to GitHub of their own accord and navigate through the files to find out.

This PR

  • Adds a github commit url helper method
  • Adds a link to the GitHub lesson links at the end of a lesson page, showing the time of last update, and linking to that lesson's commit history.

Issue

Closes #XXXXX

Additional Information

image

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this PR follows the keyword: brief description of change format, using one of the following keywords:
    • Feature - adds new or amends existing user-facing behavior
    • Chore - changes that have no user-facing value, refactors, dependency bumps, etc
    • Fix - bug fixes
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • I have verified all tests and linters pass after making these changes.
  • If this PR addresses an open issue, it is linked in the Issue section
  • If applicable, this PR includes new or updated automated tests

@MaoShizhong
Copy link
Contributor Author

For QA

  1. Navigate to any lesson or project page
  2. Scroll to the bottom where "Improve on GitHub" and "Report an Issue" links are
  3. Observe a new link indicating "Last updated ... (see changelog)".
  4. Clicking this link should navigate to the lesson file's commit history on GitHub, and the latest commit's date should match the time_ago_in_words given in the link.

@KevinMulhern
Copy link
Member

KevinMulhern commented Jun 17, 2024

This should help alleviate some confusion when something in the lesson changes when someone is midway through

Great stuff @MaoShizhong! love the problem description 🤌. I've seen this catch learners out time and time again and think this is a great solution for it.

Would this lose much of its usefulness if we dropped the date and just had "Change log"? Theres a couple of reasons why I think the date could potentially cause us problems:

  • The updated_at time stamp on records can change for many reasons unrelated to content changes. Like editing columns via seeds or data migrations. To have it 100% accurate all of the time, we'd have to do more work and do something like I described on Discord with adding a separate last_commit_date column and updating it at the same time we update the content in the import service.
  • I think we had something like this before and ended up stripping it out because we started getting questions in chat for lessons that don't change too often like this: "This lesson hasn't been updated in X years, is TOP still up to date and relevant?".

If you think including the date is absolutely necessary, thats good enough for me. But it's worth considering if its more trouble than its worth.

@MaoShizhong
Copy link
Contributor Author

@KevinMulhern Thanks for the further insights!
Just having a changelog link without the date would still be a good 80:20. My original justification for including the date was to help more with the problem highlighted about changes when mid-lesson. As in, someone could very easily just see that the lesson was last edited at X datetime, without having to navigate to another page (which they may be unfamiliar with) just to find out when something happened (no issues with needing someone to do that to find out the contents of those changes though).

But given what you said about things that could affect the date shown, I think it makes sense that we go with just the changelog link for now. If we feel adding the date in is really vital, we can look into a more accurate way of doing so, like with the last_commit_date idea of yours.

@KevinMulhern
Copy link
Member

Thanks @MaoShizhong, I can definitely see where you're coming from. Going with the simplest version for now and enhancing with the date if needed later sounds very reasonable.

@KevinMulhern KevinMulhern temporarily deployed to odin-review-app-pr-4570 June 18, 2024 15:06 Inactive
Copy link
Member

@KevinMulhern KevinMulhern left a comment

Choose a reason for hiding this comment

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

Looks great! thanks @MaoShizhong 🚀

@MaoShizhong MaoShizhong merged commit 0a3e05b into TheOdinProject:main Jun 18, 2024
3 checks passed
@MaoShizhong MaoShizhong deleted the feature/lesson-changelogs branch June 18, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants