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

Fixing timestamp issue and adding decoration #425

Merged
merged 4 commits into from
Oct 17, 2023

Conversation

M-DLB
Copy link
Collaborator

@M-DLB M-DLB commented Oct 6, 2023

This PR is fixing the problem reported here, plus additional issues (error when displaying the list of input arguments, decoration to identify the different steps of the build process).

@dennis-behm
Copy link
Member

@M-DLB Thank you for the updates.

https://github.com/M-DLB/dbb-zappbuild/blob/Fix-Timestamp/build-conf/build.properties#L108 defines also a valid time stamp. It is a bit of a "chicken and the egg" issue here, I guess, if we use the hard-coded vs the property-defined value.

Anyway, should we move to a 24-h format generally to be more consistent? We could simplify the code and remove the property.

@dennis-behm dennis-behm linked an issue Oct 9, 2023 that may be closed by this pull request
@M-DLB
Copy link
Collaborator Author

M-DLB commented Oct 9, 2023

I was not aware of this property! I think it makes sense to remove it, and only use the props.startTime to generate the build label. This would remove the buildOutputTSformat property.
What do you mean by "24-h format generally to be more consistent"?

@dennis-behm
Copy link
Member

dennis-behm commented Oct 13, 2023

Hi @M-DLB ,

What do you mean by "24-h format generally to be more consistent"?

The buildOutputTSformat has been added to use the 24 hour format yyyyMMdd.HHmmss.mmm for the build output directory, that when you sort the build output directory or the build label, it provides the correct chronological order (e.q. build.20231013.141258.012). Lower case hh in yyyyMMdd.hhmmss.mmm is the 12 hour format, leading to confusion.

I would rather go for the 24 format for all, then :-)

Signed-off-by: Mathieu Dalbin <[email protected]>
@@ -41,7 +41,7 @@ applicationDefaultPropFiles | Comma separated list of default application confi
buildListFileExt | File extension that indicates the build file is really a build list.
applicationConfRootDir | Alternate root directory for application-conf location. Allows for the deployment of the application-conf directories to a static location. Defaults to ${workspace}/${application}
createBuildOutputSubfolder | Option to create a subfolder with the build label within the build output dir (outDir). Default: true.
buildOutputTSformat | Defines the build timestamp format for build output subfolder and build label.
(Deprecated) buildOutputTSformat | Defines the build timestamp format for build output subfolder and build label.
Copy link
Member

Choose a reason for hiding this comment

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

@M-DLB Let's completely drop the buildOutputTSformat property. It is no longer used in your implementation anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, but it should be part of the changelog that this property no longer exists.

Copy link
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

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

@M-DLB Thanks for the updates. This makes it more consistent. A info message needs to be added to the release notes, that the timestamp now leverages the 24-h format for labels of the build result, the build directory (if used) etc.

@dennis-behm dennis-behm merged commit 2f125d1 into IBM:develop Oct 17, 2023
1 check passed
@M-DLB M-DLB deleted the Fix-Timestamp branch September 12, 2024 12:04
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.

Time format "yyyyMMdd.HHmmss.mmm" or "yyyyMMdd.HHmmss.SSS" ?
2 participants