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.

Sign up for GitHub

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

Jump to bottom

shellcheck all core files #1917

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

shellcheck all core files #1917

wants to merge 12 commits into from

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Aug 13, 2021 •
edited
Loading

Description

Add custom, lib, scripts, and template to clean_files.txt and address the fallout. This helps towards #1696.

Motivation and Context

My entire glob branch would have been unnecessary if these files had been checked by spellcheck, so let's do it!

How Has This Been Tested?

All tests pass!

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

@gaelicWizard
Copy link
Contributor Author

This is a draft as it is a work-in-progress!

This branch is based on uname, basename, and brew, so those PRs are assumed to have been accepted before this one. (Otherwise there are a lot of unrelated changes in here!)

NoahGorny
NoahGorny reviewed Aug 14, 2021
View reviewed changes
Copy link
Member

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

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

Wow- what a great cleanup!
I look forward for merging all of your contributions @gaelicWizard 😄

Thanks for helping make Bash-it a better project ❤️

clean_files.txt Outdated Show resolved Hide resolved
davidpfarrell
davidpfarrell suggested changes Aug 14, 2021
View reviewed changes
Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

Hi Team !

IMO, this is too much risk for one PR : 24 files?

Lets try adding lib/ and scripts/ as separate PRs and see how that looks

@gaelicWizard
Copy link
Contributor Author

IMO, this is too much risk for one PR : 24 files?

It's like six files, only one of which I've (partially) committed. The rest you see in the list are from a different PR, which has been approved but not merged yet. This PR is a draft because it's not ready yet.

Worry not, it will be clean and clear before submission! 👍

@gaelicWizard gaelicWizard force-pushed the shellcheck branch 3 times, most recently from 7b3a445 to 5ea47b7 Compare August 15, 2021 01:09
@gaelicWizard
Copy link
Contributor Author

This is a draft as it is a work-in-progress!

This branch is based on uname, basename, and brew, so those PRs are assumed to have been accepted before this one. (Otherwise there are a lot of unrelated changes in here!)

(Just rebased on master since uname has been merged.)

@gaelicWizard gaelicWizard force-pushed the shellcheck branch 2 times, most recently from 42a0e6a to 9e55192 Compare August 15, 2021 04:15
@gaelicWizard gaelicWizard force-pushed the shellcheck branch 4 times, most recently from be58f3c to 5bebe44 Compare August 26, 2021 21:18
@gaelicWizard gaelicWizard force-pushed the shellcheck branch 15 times, most recently from 75d756a to a53776f Compare September 6, 2021 03:47
@gaelicWizard gaelicWizard force-pushed the shellcheck branch 5 times, most recently from 1f201cd to ed9d71e Compare February 20, 2022 19:05
@gaelicWizard gaelicWizard changed the title DRAFT: shellcheck all core files shellcheck all core files Feb 20, 2022
@gaelicWizard gaelicWizard force-pushed the shellcheck branch 2 times, most recently from f2f34dd to f81e816 Compare February 23, 2022 23:21
@gaelicWizard gaelicWizard force-pushed the shellcheck branch 5 times, most recently from 32321bf to 2329b92 Compare March 4, 2022 20:51
gaelicWizard and others added 11 commits March 4, 2022 13:04
@gaelicWizard
install: use .bashrc and notify user
e888ddf 
The logic to guess whether to use `.bash_profile` or `.bashrc` was buggy and wrong. Just use `.bashrc` and either automatically fill in a `.bash_profile`, or notify the user that they need to edit their `.bash_profile`.
@gaelicWizard
install: cleanup
0853724
@gaelicWizard
uninstall: cleanup
c3de63b
@gaelicWizard
docs: fix references to ~/.bashrc
d55adc6 
docs/installation: add to note about interactive/login shells
@gaelicWizard
install: _bash-it-install-modify-profile()
4c5e87e
@gaelicWizard
uninstall: //echo/printf
0485778 
- Alsö, add implementation note at top.
@gaelicWizard
uninstall: try to determine initialization file
68149f7 
uninstall: TIL that `fgrep` is deprecated...
@gaelicWizard
template: shfmt
52cd75b
@gaelicWizard
template: shellcheck
affbce6 
- and generally comment out useless varbls
@gaelicWizard
lint: add lib to clean_files.txt
8a6debd
@gaelicWizard
Revert "bash_it: source reloader.bash without arguments for the defau…
c09c050 
…lt enabling"

This reverts commit e05fa47.
This reverts commit ee85367.
@seefood
Copy link
Contributor

seefood commented Nov 10, 2024

Looks ready to go. just a merge from master, please.

@seefood seefood added seems abandoned rattle the cage, and close if nobody wants to keep it going waiting-for-response labels Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NoahGorny NoahGorny Awaiting requested review from NoahGorny

@davidpfarrell davidpfarrell Awaiting requested review from davidpfarrell

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
seems abandoned rattle the cage, and close if nobody wants to keep it going waiting-for-response
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gaelicWizard @seefood @NoahGorny @davidpfarrell