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

Update New-MDTable.ps1 #27

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Update New-MDTable.ps1 #27

merged 1 commit into from
Jun 6, 2024

Conversation

aaronparker
Copy link
Contributor

Fix missing colon in Left separator.

Existing code does not add the left aligned colon to left align text, e.g. in the command below, the left aligned column is missing the colon character

Get-Command New-MDTable | New-MDTable -Columns ([ordered]@{Name="left";CommandType="center";Version="right"})
| Name        | CommandType | Version |
| ----------- |:-----------:| -------:|
| New-MDTable | Function    | 1.9     |

Fix missing colon in Left separator
@AppVeyorBot
Copy link

@Sarafian
Copy link
Owner

Sarafian commented May 6, 2024

Hi @aaronparker
The fix is very simple and makes sense. Not sure if you ran the unit tests, but the tests should break with the one New-MDTable with ordered columns because for some reason the : is also forgotten there.

Unfortunately I've not worked with powershellk for a while and the project's unit testing is outdated. There is another pull request #25 which I guess should solve the issues but it has not been merged.

If I can ask for the favour, would it possible to check #25 and inrcorporate those changes so the repository/module could pass all tests and get published?

@Sarafian Sarafian mentioned this pull request May 29, 2024
@Sarafian
Copy link
Owner

@aaronparker I merged the #25 so if you can pull the changes from the master, and make sure that the Invoke-Test.ps1 works, then it should pass the test.

Before it is merged, the minor version needs to bumped to 1.20 and a line added in the changelog file. Not sure how I could contribute to be honest. I've not worked for ages on Github.

@Sarafian Sarafian added the bug label May 31, 2024
@Sarafian Sarafian changed the base branch from master to 42-alignment-issue-for-left-with-new-mdtable June 6, 2024 16:15
@Sarafian Sarafian merged commit bd13aae into Sarafian:42-alignment-issue-for-left-with-new-mdtable Jun 6, 2024
1 check failed
@Sarafian
Copy link
Owner

Sarafian commented Jun 6, 2024

Accepting the code fix and continue with GH-42 to incorporate the rest of the changes. Thank you @aaronparker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants