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 docs and scripts concerning contributing and setup #3300

Merged
merged 1 commit into from
Jan 3, 2023

Conversation

martinemde
Copy link
Member

@martinemde martinemde commented Dec 16, 2022

Since I'm coming into the repository with fresh eyes, I'm passing along the things I felt could be clarified.

@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Merging #3300 (8f8c7af) into master (42f3237) will decrease coverage by 0.05%.
The diff coverage is n/a.

❗ Current head 8f8c7af differs from pull request most recent head 9143fca. Consider uploading reports for the commit 9143fca to get more accurate results

@@            Coverage Diff             @@
##           master    #3300      +/-   ##
==========================================
- Coverage   98.57%   98.52%   -0.06%     
==========================================
  Files         113      113              
  Lines        3372     3390      +18     
==========================================
+ Hits         3324     3340      +16     
- Misses         48       50       +2     
Impacted Files Coverage Δ
app/models/concerns/rubygem_searchable.rb 94.44% <0.00%> (-5.56%) ⬇️
lib/elastic_searcher.rb 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@martinemde martinemde force-pushed the martinemde/contributing-and-setup branch 2 times, most recently from 17224a4 to 8f8c7af Compare December 19, 2022 21:30
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
script/setup Outdated
Comment on lines 2 to 4
echo "DEPRECATED: This script has moved to bin/setup and no longer drops the database."
echo "Running bin/setup"
exec "$(dirname "$0")/../bin/setup"
Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure about offering a message like this to help direct to bin/setup vs deleting script/setup entirely. Given the behavior was duplicated for the last 3+ years, I figure leaving a helpful message here is no big deal.

Copy link
Member

Choose a reason for hiding this comment

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

If the file is useless and is not part of the docs anymore, I think we can just remove it. There is no need for deprecation in here.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@martinemde martinemde force-pushed the martinemde/contributing-and-setup branch from 8f8c7af to 6b514d5 Compare January 3, 2023 18:48
Use newer bin/setup instead of script/setup; remove script/setup
bin/setup does everything from script/setup besides db:drop which can be
manually run when needed.
@martinemde martinemde force-pushed the martinemde/contributing-and-setup branch from 6b514d5 to 9143fca Compare January 3, 2023 18:51
@martinemde
Copy link
Member Author

Review comments addressed. Should be ready to go if there are on objections.

@martinemde martinemde merged commit 273acd5 into master Jan 3, 2023
@martinemde martinemde deleted the martinemde/contributing-and-setup branch January 3, 2023 19:02
@rubygems-org-shipit rubygems-org-shipit bot temporarily deployed to staging January 3, 2023 19:14 Inactive
@rubygems-org-shipit rubygems-org-shipit bot temporarily deployed to production January 18, 2023 15:08 Inactive
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