Skip to content

Feat: Display Tag in Commit Details#20

Open
ancy-augustin wants to merge 3 commits into
masterfrom
users/ancy/feature/display-git-tag
Open

Feat: Display Tag in Commit Details#20
ancy-augustin wants to merge 3 commits into
masterfrom
users/ancy/feature/display-git-tag

Conversation

@ancy-augustin

@ancy-augustin ancy-augustin commented Oct 25, 2024

Copy link
Copy Markdown
Collaborator

What does this pull request accomplish?

The PR adds functionality to display the tag (if exists) along with Commit Details.

Implementation Details

Implemented by checking the existence of tags. If tags found, the latest tag is retrieved and displayed in the Commit Details.

Files Changed

  • Code Compare Tool For Git.vi

Why should this pull request be merged?

Merging this PR allows the user to get the tag details (if exists) thus improving user experience.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@SatheeshvarmaSJ SatheeshvarmaSJ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a test to see if the review is working.

@ancy-augustin ancy-augustin changed the title Feature: Display Tag in Commit Details Feat: Display Tag in Commit Details Oct 28, 2024
@KarthikAbiram

Copy link
Copy Markdown
Collaborator

@SatheeshvarmaSJ Is this tested, reviewed and ready to merge?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is a constant, this VI can be inline.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is a constant, this VI can be inline.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As per this comment in the Exit case, the VI title should not have the beta tag added and saved.
image

Just leave the title as below
image

Instead of
image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The beta tag is removed in Exit case and VI Properties and saved.
But in Initilaize case, the Beta tag is present with a note as below
image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The entire tab control seems to be shifted compared to the previous version. This makes the tab titles slightly clip into the title bar. Refer the previous version for the size and position and adjust.
image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Adjusted the position.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Better to not save the dropdowns with the details. Clear them while saving the VI.
image
image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Cleared the drop down contents

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The default visible case is different than Initialize (or a significant case). Better to have the default case to Initialize or any one of the most important/frequently accessed case.
image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Initialize case is set to default visible case

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Previously existing new line is missing.
image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reverted to the previously existing format string and inserted the tag details as specified in the prototype
image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Build the string using the newly created constant instead of hardcoding.
image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed the comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

major: The Tag always displays the latest tag available in the branch and not specific to the selected commit

This commit should display the tag as Tag: v1.1.0.2
image

image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Modified to display the tag commit specific

@SatheeshvarmaSJ

Copy link
Copy Markdown
Contributor

@SatheeshvarmaSJ Is this tested, reviewed and ready to merge?

I have reviewed and requested changes @KarthikAbiram

@ancy-augustin

Copy link
Copy Markdown
Collaborator Author

Addressed the review comments. Kindly have a look and share your feedback @SatheeshvarmaSJ

@KarthikAbiram

Copy link
Copy Markdown
Collaborator

@SatheeshvarmaSJ Please let me know if these can be merged

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.

LabVIEW Code Compare Tool for Git: Display git tag (if it exists) in the Commit Details

3 participants