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

style: Remove trailing whitespace and missing final newlines #433

Merged
merged 5 commits into from
Apr 17, 2023

Conversation

hyperupcall
Copy link
Contributor

@hyperupcall hyperupcall commented Apr 7, 2023

In #272, one section stated to "delete trailing spaces". This does that, and also missing final newlines (so files are files by POSIX)

This also integrates editorconfig-checker with GitHub Actions so the style will continue to remain consistent. It runs only on files that have changed for speed. I used editorconfig-checker because integrating shfmt might have changed more lines than what might be considered necessary; editorconfig-checker checks the bare minimum.

The only real code change resulting from this is in tools/uninstall.sh

Copy link
Contributor

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

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

Thank you for your work!

I actually left these tasks in #272 to avoid possible conflicts with existing PRs, but I think now it is time to actively work on them. When I took over the maintenance of this project, there were about 50 or more pending PRs, which I hadn't time to look into. But we have already processed most of the important PRs that existed at that time.


Maybe we do not have to decide it in this PR, but another thing that I might need to rethink is the format of the commit message. There were really no explicit rules for the format of the commit message, but as for the first line, I was loosely following the suggestion made in CONTRIBUTING.md of #90.

<FILE>: <Summary>

<Description>
<Fixes/Bug>: #<BUG_NUMBER>
Signed-off-by: <name> <surname> <e-mail>

However, I actually have no plan to merge #90. Maybe I would pick some changes from #90, but I probably reject most of the changes suggested in #90.

Also, personally, I'd like to switch to Conventional Commits as in this PR. What do you think?

edit: I now noticed #86, which describes a proposal for the beginning line of the commit message in detail.

.git-blame-ignore-revs Outdated Show resolved Hide resolved
tools/uninstall.sh Outdated Show resolved Hide resolved
tools/uninstall.sh Outdated Show resolved Hide resolved
tools/uninstall.sh Outdated Show resolved Hide resolved
@akinomyoga
Copy link
Contributor

@hyperupcall

@akinomyoga akinomyoga force-pushed the whitespace branch 2 times, most recently from 00f5969 to 70be067 Compare April 16, 2023 08:48
@hyperupcall
Copy link
Contributor Author

hyperupcall commented Apr 16, 2023

I actually left these tasks in #272 to avoid possible conflicts with existing PRs, but I think now it is time to actively work on them. When I took over the maintenance of this project, there were about 50 or more pending PRs, which I hadn't time to look into. But we have already processed most of the important PRs that existed at that time.

👍

Also, personally, I'd like to switch to Conventional Commits as in this PR. What do you think?

I think Conventional Commits is good to use - it's what is used at asdf too, but we might need to add a few extra things. I looked at #86, and I think it's heading in the right direction, but I would add a few more things. For example, it suggests something like git-avh: Doing something here or completions: Fixed #GITHUB-ISSUE-NUMBER for a git-avh related completion feature. But, I would do something like completions(git-avh): Doing something here #GITHUB-ISSUE-NUMBER

I do apologize for my late reply, I know I started work on this a week ago

@hyperupcall
Copy link
Contributor Author

I looked things over and the new changes look good to me! I'll look over #432 tomorrow morning

@akinomyoga
Copy link
Contributor

Thanks!

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.

2 participants