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

feat(webapp): check if a new version is available #757

Merged
merged 10 commits into from
Mar 8, 2024

Conversation

irvingoujAtDevolution
Copy link
Contributor

Added tooltip. Changed the interceptor logic, now, it will not add auth header for all request to third party url

@CBenoit
Copy link
Member

CBenoit commented Mar 8, 2024

Is there a Jira ticket for this? This should be added in the final commit as a footer like so:

image

This commit should also follow the conventional commit specification.

If all of this is done properly, the change will be properly documented automatically in our changelog when it’s time to cut a new release.

For example:

image

Failing at following this convention is making it harder to maintain the changelog properly.

@CBenoit CBenoit changed the title Add version update icon and link when newer version is available feat(webapp): check if a new version is available Mar 8, 2024
@CBenoit
Copy link
Member

CBenoit commented Mar 8, 2024

I modified the title of the PR to demonstrate how it should look like in the interest of generating a good changelog:

feat(webapp): check if a new version is available

  • It’s a new feature
  • For the web application
  • What is the feature in a few words, the user don’t need to know implementation details

If more details should be mentioned, they can be written down in the body of the commit. This will be part of the changelog too.
Try to not include all the intermediate commit messages, because I’ll have to remove this from the changelog manually later.
GitHub automatically includes them when squashing a PR with many commits:
image

This should look like this:
image

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

The patch looks good to me, but maybe @kristahouse can have a deeper look?
My comments above are mostly on the form. Feel free to merge, but try to follow the convention to ease the future releases. Thank you!

@irvingoujAtDevolution
Copy link
Contributor Author

irvingoujAtDevolution commented Mar 8, 2024

The patch looks good to me, but maybe @kristahouse can have a deeper look? My comments above are mostly on the form. Feel free to merge, but try to follow the convention to ease the future releases. Thank you!

Will do, I used github cli to automatically fills up the title, used it wrong obviously. Will pay extra attention on it next time.

@irvingoujAtDevolution irvingoujAtDevolution merged commit d2d8811 into master Mar 8, 2024
16 checks passed
@irvingoujAtDevolution irvingoujAtDevolution deleted the Add-version branch March 8, 2024 16:50
@kristahouse
Copy link
Contributor

Just to follow up. The patch LGTM! Thanks @irvingoujAtDevolution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants