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

Adds github workflow ci action #5

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

warmwaffles
Copy link
Contributor

This is to replace Travis.

I'm working on another branch to modernize this up to latest Elixir and OTP

@warmwaffles
Copy link
Contributor Author

warmwaffles commented Nov 28, 2023

I think supporting the latest 3 versions of OTP and elixir are preferable. As anything beyond is unsupported by upstream erlang and elixir doesn't push updates for older versions.

mix.exs Outdated
docs: ["docs", &copy_doc_to_docs/1]
docs: ["docs", &copy_doc_to_docs/1],
lint: [
"format --check-formatted",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Format fails because the .formatter.exs is not present at the project root level. And if you add one, it causes a ton of file churn. Not sure if we want to keep this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If the current indentation and formatting is good and clear, we can keep it like as is. If at some point you consider that would be an improvement, by applying an automatic formatter (like styler), go for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can probably drop it. It'll add file churn that will muddy the blame up.

@rNoz
Copy link
Collaborator

rNoz commented Nov 29, 2023

Good one! Thanks!

@warmwaffles warmwaffles force-pushed the github-actions branch 2 times, most recently from bf08877 to b9d6dde Compare November 29, 2023 14:24
@warmwaffles
Copy link
Contributor Author

I need #7 to land before this one does. There's some funkyness with credo.

@rNoz
Copy link
Collaborator

rNoz commented Dec 6, 2023

Other PRs are squashed+merged.

Some of these CI tasks are failing due to a nested dependency (JSX).

@warmwaffles
Copy link
Contributor Author

Awesome, rebased and now just the linter fails. Will get that rectified.

@warmwaffles
Copy link
Contributor Author

warmwaffles commented Dec 6, 2023

@rNoz linter fixed and now everything passes.

EDIT: I also agree with the squash merge philosophy for libraries. Keeps the history nice and linear.

@rNoz rNoz merged commit ba2210a into Group4Layers:master Dec 6, 2023
9 checks passed
@warmwaffles warmwaffles deleted the github-actions branch December 6, 2023 19:07
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