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

Ci Fixes #1336

Merged
merged 4 commits into from
Apr 29, 2024
Merged

Ci Fixes #1336

merged 4 commits into from
Apr 29, 2024

Conversation

Judahmeek
Copy link
Collaborator

@Judahmeek Judahmeek commented Apr 13, 2024

This PR addresses linting and some other CI issues in preparation for #1335

@Judahmeek Judahmeek force-pushed the judahmeek/ci-sanity-check branch from 4ca5db5 to 081e33c Compare April 13, 2024 02:51
@Judahmeek Judahmeek changed the title Fake PR to compare against #1335 Ci Fixes Apr 13, 2024
@Judahmeek Judahmeek requested a review from ahangarha April 13, 2024 02:57
@Judahmeek Judahmeek marked this pull request as ready for review April 13, 2024 02:58
@@ -89,7 +89,7 @@ jobs:
with:
persist-credentials: false
- uses: actions/setup-node@v3
- run: npm -g install yalc ${{ matrix.js_package_manager.installer }}
- run: sudo npm -g install yalc ${{ matrix.js_package_manager.installer }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems weird that we have to use sudo but not that concerned since this in a temp CI runner 😁

(I figure it might be related to supporting old versions of Node and what not)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The sudo is to resolve a EACCES: permission denied error that I was getting.

The other option I saw in my search was --unsafe-perm, but that's apparently obsolete.

@@ -4,3 +4,6 @@ source "http://rubygems.org"

gemspec
# This is an optional dev-dependency, required whenever sprockets is required
gem "rubocop"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not in the gemspec as a development dependency?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because CI doesn't install development dependencies from the gemspec.

Do you want me to create a Gemfile.development_dependencies file like react_on_rails has?

@justin808 justin808 merged commit 543b0e3 into master Apr 29, 2024
19 checks passed
@justin808 justin808 deleted the judahmeek/ci-sanity-check branch April 29, 2024 06:32
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.

3 participants