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.

Sign up for GitHub

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

Jump to bottom

uci2: update revision #15990

Merged
merged 1 commit into from
Jun 28, 2021
Merged

uci2: update revision #15990

merged 1 commit into from
Jun 28, 2021

Conversation

jpetrina
Copy link
Contributor

Pull in new API function from upstream.

Signed-off-by: Jakov Petrina jakov.petrina@sartura.hr

@jpetrina
uci2: update revision
e779323 
Pull in new API function from upstream.

Signed-off-by: Jakov Petrina <jakov.petrina@sartura.hr>
@neheb neheb merged commit c78dc17 into openwrt:master Jun 28, 2021
@BKPepe
Copy link
Member

BKPepe commented Jun 29, 2021

I am reverting this commit for the following reasons:

  1. You are pulling different sources, but you didn't update the PKG_VERSION or even PKG_RELEASE. This ends up in a situation when existing users who have installed your library will have a different version than users who installed it after this PR.

  2. Please, don't use cryptic commit messages like "update revision". These are not helping to know others to know something more while going through history. If you are using the same commit subject, then you will be lost. If possible specific to which version you do the bump.

BKPepe added a commit that referenced this pull request Jun 29, 2021
@BKPepe
Revert "uci2: update revision"
39dedfe 
I'm reverting this commit, because it is pulling newer source code, but
without doing a new version bump. This results that users who already
installed this library on their router will have different package
content than users who installed it after the commit.

The another small reason is that commit subject - update revision is cryptic itself and should be more bulletproof like saying update to version 1.1

More details: #15990 (comment)

This reverts commit e779323.

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
@jpetrina
Copy link
Contributor Author

@BKPepe Thanks, I agree with necessity of either PKG_VERSION or PKG_RELEASE. I suppose PKG_RELEASE would be appropriate here until upstream decides to release v1.1

Regarding cryptic commit messages, would using Git commit hashes be appropriate e.g. uci2: bump to revision ccaf677 or perhaps something like uci2: bump to v1.0-ccaf677?

@BKPepe
Copy link
Member

BKPepe commented Jun 29, 2021

Fine by me.

1715173329 pushed a commit to immortalwrt/packages that referenced this pull request Jun 30, 2021
@BKPepe @1715173329
Revert "uci2: update revision"
8e3e68a 
I'm reverting this commit, because it is pulling newer source code, but
without doing a new version bump. This results that users who already
installed this library on their router will have different package
content than users who installed it after the commit.

The another small reason is that commit subject - update revision is cryptic itself and should be more bulletproof like saying update to version 1.1

More details: openwrt/packages#15990 (comment)

This reverts commit e779323.

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Signed-off-by: Tianling Shen <cnsztl@immortalwrt.org>
@pprindeville
Copy link
Member

@BKPepe Thanks, I agree with necessity of either PKG_VERSION or PKG_RELEASE. I suppose PKG_RELEASE would be appropriate here until upstream decides to release v1.1

Regarding cryptic commit messages, would using Git commit hashes be appropriate e.g. uci2: bump to revision ccaf677 or perhaps something like uci2: bump to v1.0-ccaf677?

Don't forget that dates are always an option too, like uci2: bump to head as of 2021-12-15 or whatever...

@jpetrina jpetrina mentioned this pull request Jul 14, 2021
Merged
@jpetrina jpetrina deleted the uci2-bump branch July 14, 2021 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jpetrina @BKPepe @pprindeville @neheb