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

Remove Swiftlint from Dockerfile #268

Merged
merged 1 commit into from
Sep 13, 2019
Merged

Conversation

f-meloni
Copy link
Member

@f-meloni f-meloni commented Sep 6, 2019

Danger-Swift takes quite a lot when runs on GitHub Actions
Schermata 2019-09-06 alle 22 48 12
Approximately more than half of that time it to clone swiftlint, that not everyone uses, and someone uses from SPM.

@f-meloni
Copy link
Member Author

f-meloni commented Sep 6, 2019

Schermata 2019-09-06 alle 23 12 17

This is this branch, with all the dev dependencies, then it could be quite faster

@f-meloni
Copy link
Member Author

f-meloni commented Sep 6, 2019

@orta do you have an opinion about this?

@f-meloni
Copy link
Member Author

f-meloni commented Sep 7, 2019

I saw that there is an option to have inputs on Actions, we could explore this later to re add in an optional way

@orta
Copy link
Member

orta commented Sep 7, 2019

Yeah, that's what I was going to recommend (using the input to determine it)

I feel like lots of people would use it with SwiftLint - maybe we could have two dockerfiles?

@f-meloni f-meloni force-pushed the remove_swiftlint_from_actions branch from 0915e4b to c87b14c Compare September 7, 2019 23:41
@f-meloni
Copy link
Member Author

f-meloni commented Sep 8, 2019

Ok, I've done few tests, and looks like the env variables are set after the build, then it can not work unless I put it on the ENTRYPOINT, that is not a good idea.
@orta can you give me more details about what you were thinking with the two Dockerfiles?
Because I think we can have only one, but we can have another repo with a Dockerfile that contains swiftlint maybe?

@f-meloni f-meloni force-pushed the remove_swiftlint_from_actions branch from 7c27450 to 4f4db24 Compare September 8, 2019 00:42
@orta
Copy link
Member

orta commented Sep 13, 2019

It'd need to be a special branch or tag per release with a different dockerfile. Totally scriptable on a release.

@f-meloni
Copy link
Member Author

Amazing idea! Let's merge this then I will create something with Rocket.
We could keep two Dockerfile and make two different tags in two different comments and then restore everything before we restore the dependencies

@f-meloni f-meloni merged commit 25d2b55 into master Sep 13, 2019
@f-meloni f-meloni deleted the remove_swiftlint_from_actions branch September 13, 2019 20:52
@litso
Copy link

litso commented Jan 3, 2020

@f-meloni Do you have any tips on how to install swiftlint on the docker image now that it is removed?

@f-meloni
Copy link
Member Author

f-meloni commented Jan 3, 2020

@litso there are few ways of doing it, the main ones I can think:

  • Change the Rocket configuration of this repo to replace the Dockerfile with another Dockerfile that has swiftlint, create a $version-swiflint tag and then release restore the Dockerfile.
    Given we use Rocket to release that will make a release with a Dockerfile that contains swiftlint for each release we make
  • You can create your own Dockerfile like https://github.com/f-meloni/danger-swift-swiftlint-action

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.

None yet

3 participants