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

Add dev scripts to bin directory on install #1491

Merged
merged 13 commits into from
Jan 9, 2023
Merged

Add dev scripts to bin directory on install #1491

merged 13 commits into from
Jan 9, 2023

Conversation

ahangarha
Copy link
Contributor

@ahangarha ahangarha commented Oct 26, 2022 •
edited by justin808
Loading

Add handy script in bin directory to run Procfiles

Closes #1482

This change is Reviewable

@ahangarha ahangarha force-pushed the add-dev-scripts-in-bin branch from e36fd8c to d4cc4f2 Compare October 26, 2022 11:27
@ahangarha ahangarha changed the title Add dev scripts to bin directory on install WIP - Add dev scripts to bin directory on install Oct 26, 2022
@ahangarha
Copy link
Contributor Author

Some tests fail but not because of my changes.

@ahangarha ahangarha changed the title WIP - Add dev scripts to bin directory on install Add dev scripts to bin directory on install Oct 26, 2022
@ahangarha ahangarha force-pushed the add-dev-scripts-in-bin branch from f6fce8d to 4775d8b Compare October 26, 2022 15:21
justin808
justin808 reviewed Oct 27, 2022
View reviewed changes
@ahangarha ahangarha force-pushed the add-dev-scripts-in-bin branch from 00dac2b to 8850be6 Compare October 27, 2022 11:05
@ahangarha ahangarha requested a review from justin808 November 24, 2022 19:36
@justin808
Copy link
Member

Please add a CHANGELOG entry and I'll merge.

@ahangarha ahangarha mentioned this pull request Dec 5, 2022
Merged
Judahmeek
Judahmeek approved these changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@Judahmeek Judahmeek left a comment

Choose a reason for hiding this comment

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

LGTM

@Judahmeek Judahmeek force-pushed the add-dev-scripts-in-bin branch from 0ad9d42 to 4b99a31 Compare December 8, 2022 01:07
@justin808
Copy link
Member

@Judahmeek and @ahangarha, I'd like to merge, but we need to fix CI failures first.


An error occurred while installing psych (5.0.0), and Bundler cannot continue.

In Gemfile:
  sdoc was resolved to 2.4.0, which depends on
    rdoc was resolved to 6.5.0, which depends on
      psych
Error: Process completed with exit code 5.

@ahangarha
Copy link
Contributor Author

I'd like to merge, but we need to fix CI failures first.

@justin808 The issue comes from psych v5+ and in particular from Related this PR: ruby/psych#541.

Should we install libyaml-dev directly in our CI?

I opened an issue on their repo to get their insight.

@ahangarha ahangarha mentioned this pull request Dec 20, 2022
Merged
@ahangarha ahangarha force-pushed the add-dev-scripts-in-bin branch from b37463d to 16cdead Compare December 30, 2022 17:22
@ahangarha
Copy link
Contributor Author

@justin808
Now not only we prefer Overmind over Foreman, but also the script is written in Ruby so it is expected to work cross platform.

@ahangarha ahangarha force-pushed the add-dev-scripts-in-bin branch from 6b0c056 to 7e84926 Compare December 31, 2022 17:13
@ahangarha ahangarha changed the title Add dev scripts to bin directory on install WIP - Add dev scripts to bin directory on install Dec 31, 2022
@ahangarha
Copy link
Contributor Author

@justin808
Shouldn't we add these scripts to the dummy app as well?
I think we should.

@ahangarha ahangarha changed the title WIP - Add dev scripts to bin directory on install Add dev scripts to bin directory on install Jan 4, 2023
@Judahmeek
Copy link
Contributor

@ahangarha can you squash this & rebase it?

We should be able to merge it after that.

@ahangarha ahangarha force-pushed the add-dev-scripts-in-bin branch from 2280cee to 66529fa Compare January 7, 2023 09:14
@ahangarha
Copy link
Contributor Author

@Judahmeek
It was challenging to fix the mess I made in the commit history, but I think I did it well.
I think this PR is ready to be merged.

Judahmeek
Judahmeek approved these changes Jan 7, 2023
View reviewed changes
Copy link
Contributor

@Judahmeek Judahmeek left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 4 files at r1, 9 of 9 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ahangarha and @justin808)

@ahangarha ahangarha requested review from justin808 and removed request for justin808 January 8, 2023 05:14
@Judahmeek Judahmeek marked this pull request as draft January 9, 2023 00:38
@Judahmeek Judahmeek marked this pull request as ready for review January 9, 2023 00:39
justin808
justin808 approved these changes Jan 9, 2023
View reviewed changes
@justin808 justin808 merged commit 6983a94 into master Jan 9, 2023
@justin808 justin808 deleted the add-dev-scripts-in-bin branch January 9, 2023 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justin808 justin808 justin808 approved these changes

@Judahmeek Judahmeek Judahmeek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Install script should create ./bin/dev
3 participants
@ahangarha @justin808 @Judahmeek