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

fix: 24h time #526

Merged
merged 1 commit into from
Jun 2, 2023
Merged

fix: 24h time #526

merged 1 commit into from
Jun 2, 2023

Conversation

Senna46
Copy link
Collaborator

@Senna46 Senna46 commented Jun 2, 2023

No description provided.

@@ -76,7 +76,7 @@ <h2 class="card-title"></h2>
{{ block?.block?.header?.proposer_address }}
</td>
<td>{{ block?.block?.data?.txs?.length }}</td>
<td>{{ block?.block?.header?.time | date : 'yy/MM/dd hh:mm' }}</td>
<td>{{ block?.block?.header?.time | date : 'yy/MM/dd HH:mm' }}</td>
</tr>
</ng-container>
<ng-template #empty>
Copy link

Choose a reason for hiding this comment

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

The code patch appears to be a simple change in the format of displaying a timestamp. The 'hh:mm' format has been changed to 'HH:mm'. This change may indicate that the timestamps are now expected to be displayed in a 24-hour clock format instead of a 12-hour format.

As this is just a minor change, there is no significant bug risk associated with it. However, it might be useful to review how the timestamps are being generated and ensure that they are being interpreted correctly after this change.

Overall, there are no improvement suggestions specifically related to this code patch as it achieves its intended purpose in a straightforward manner.

@@ -42,7 +42,7 @@ <h2 class="card-title">Latest Blocks</h2>
{{ block?.block?.header?.proposer_address }}
</td>
<td>{{ block?.block?.data?.txs?.length }}</td>
<td>{{ block?.block?.header?.time | date : 'yy/MM/dd hh:mm' }}</td>
<td>{{ block?.block?.header?.time | date : 'yy/MM/dd HH:mm' }}</td>
</tr>
</ng-container>
<ng-template #empty>
Copy link

Choose a reason for hiding this comment

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

The code patch changes the time format in a table cell from 'yy/MM/dd hh:mm' to 'yy/MM/dd HH:mm'. The change replaces the lower-case "hh" (hour in 12-hour clock format) with upper-case "HH" (hour in 24-hour clock format). This is a minor change and doesn't seem to introduce any bug risks. However, it could be an improvement if the goal is to ensure that the time format is consistently displayed as 24-hour time across the application.

@Senna46 Senna46 merged commit b28cc09 into main Jun 2, 2023
@Senna46 Senna46 deleted the hotfix/time-24h branch June 2, 2023 11:42
@@ -43,7 +43,7 @@ <h2 class="card-title">Transactions</h2>
{{ tx.txhash }}
</th>
<td>
{{ tx.timestamp | date : 'yy/MM/dd hh:mm' }}
{{ tx.timestamp | date : 'yy/MM/dd HH:mm' }}
</td>
</tr>
</ng-container>
Copy link

Choose a reason for hiding this comment

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

The code patch you provided is a git diff which shows a change in a specific line of code within an HTML template file.

The change modifies the formatting of the timestamp displayed in the template, specifically changing the format from "yy/MM/dd hh:mm" to "yy/MM/dd HH:mm". The former uses the 12-hour clock while the latter uses the 24-hour clock.

From a bug risk perspective, there does not seem to be any issue caused by this code change, as it only affects the display of the timestamp and does not affect any underlying functionality.

As for improvement suggestions, it depends on the context and requirements of the application. If the use case demands a specific timestamp format, then the change made here may be appropriate. However, if the timestamp format is flexible and can be customized by the user, it may be better to provide options for both 12-hour and 24-hour formats or even give a variety of other format options.

@@ -72,7 +72,7 @@
{{ tx.txhash }}
</th>
<td>
{{ tx.timestamp | date : 'yy/MM/dd hh:mm' }}
{{ tx.timestamp | date : 'yy/MM/dd HH:mm' }}
</td>
</tr>
</ng-container>
Copy link

Choose a reason for hiding this comment

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

This code patch seems to be a simple change in the formatting of a date/time display. The change is from using lowercase 'hh' (which represents 12-hour clock format) to uppercase 'HH' (which represents 24-hour clock format). This change may improve consistency if the rest of the code also uses 24-hour clock format, but it's not likely to introduce any bugs or risks by itself.

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.

1 participant