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

Bump to opensearch-dashboards-1.3.1 App Version in helm release chart version 1.4.0 #246

Merged
merged 3 commits into from
Apr 7, 2022
Merged

Bump to opensearch-dashboards-1.3.1 App Version in helm release chart version 1.4.0 #246

merged 3 commits into from
Apr 7, 2022

Conversation

abhinavGupta16
Copy link
Contributor

Signed-off-by: Abhinav Gupta abhng@amazon.com

Description

Added changelog for 1.3.1 opensearch-dashboards release

Issues Resolved

opensearch-project/opensearch-build#1885

Check List

  • Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • Helm chart version bumped
  • Helm chart CHANGELOG.md updated to reflect change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Abhinav Gupta <abhng@amazon.com>
@abhinavGupta16 abhinavGupta16 requested a review from a team as a code owner April 7, 2022 20:00
@abhinavGupta16 abhinavGupta16 self-assigned this Apr 7, 2022
@prudhvigodithi
Copy link
Member

Hey @abhinavGupta16 please change appVersion, to point to the latest version for dashboard.

Signed-off-by: Abhinav Gupta <abhng@amazon.com>
Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

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

This is new release means minor bump

Signed-off-by: Abhinav Gupta <abhng@amazon.com>
@peterzhuamazon peterzhuamazon merged commit 414126c into opensearch-project:main Apr 7, 2022
@sastorsl
Copy link
Contributor

sastorsl commented Apr 8, 2022

Could the title of this PR be updated to reflect show that the actual version bump is the appVersion?
"Bump to opensearch-dashboards-1.3-1 in helm release 1.4.0" for instance?

@TheAlgo
Copy link
Member

TheAlgo commented Apr 8, 2022

I agree with @sastorsl this commit message is not so good even if the version number was correct. Here the intent is to not add CHANGELOG but bump the version.

IMHO I don't prefer commits which has a past tense (Eg : Bumped , Added). If we see the default git commit those are always like commands (Eg : Merge abc from cde)

@TheAlgo
Copy link
Member

TheAlgo commented Apr 8, 2022

Also @abhinavGupta16 in the issue attached I could find the details regarding helm update. There is no mention about the release in helm charts

@abhinavGupta16 abhinavGupta16 deleted the abhng-1.3.1-dashboards-update branch April 8, 2022 09:20
@abhinavGupta16 abhinavGupta16 changed the title added changelog for 1.3.1 opensearch-dashboards added changelog for 1.3.1 opensearch-dashboards for helm 1.4.0 Apr 8, 2022
@abhinavGupta16
Copy link
Contributor Author

abhinavGupta16 commented Apr 8, 2022

I agree with @sastorsl this commit message is not so good even if the version number was correct. Here the intent is to not add CHANGELOG but bump the version.

IMHO I don't prefer commits which has a past tense (Eg : Bumped , Added). If we see the default git commit those are always like commands (Eg : Merge abc from cde)

Well, I don't think tense matters as long as the message is conveyed.

@abhinavGupta16
Copy link
Contributor Author

Could the title of this PR be updated to reflect show that the actual version bump is the appVersion? "Bump to opensearch-dashboards-1.3-1 in helm release 1.4.0" for instance?

Good suggestion. I updated the title. It would make it easier to search for the PR

@TheAlgo
Copy link
Member

TheAlgo commented Apr 8, 2022

Could the title of this PR be updated to reflect show that the actual version bump is the appVersion? "Bump to opensearch-dashboards-1.3-1 in helm release 1.4.0" for instance?

Good suggestion. I updated the title. It would make it easier to search for the PR

@abhinavGupta16 Do you mind changing to Bump appversion to 1.3.1 or Change default version to 1.3.1 instead of added changelog?

@TheAlgo
Copy link
Member

TheAlgo commented Apr 12, 2022

@abhinavGupta16 Any update on this?

@peterzhuamazon peterzhuamazon changed the title added changelog for 1.3.1 opensearch-dashboards for helm 1.4.0 Bump to opensearch-dashboards-1.3-1 in helm release 1.4.0 Apr 12, 2022
@peterzhuamazon
Copy link
Member

peterzhuamazon commented Apr 12, 2022

I changed it.

 Bump to opensearch-dashboards-1.3.1 App Version in helm release chart version 1.4.0

Thanks @sastorsl @TheAlgo

@peterzhuamazon peterzhuamazon changed the title Bump to opensearch-dashboards-1.3-1 in helm release 1.4.0 Bump to opensearch-dashboards-1.3.1 App Version in helm release chart version 1.4.0 Apr 12, 2022
@sastorsl
Copy link
Contributor

Well, I don't think tense matters as long as the message is conveyed.

It's a frequent discussion topic, and of course opinions matter on this subject.
If git itself can be a guide it mentions this in its documentation, and favours the "imperative" / present tense style.

The most important though is that it is maintained throughout the project, so I would suggest that the OpenSearch-team opens this up for discussion in a separate issue and make a decision.

More importantly, thank you for updating the title :-)

This was referenced Jul 16, 2023
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.

5 participants