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

Travis: line length tweaks #128

Merged
merged 2 commits into from
Jan 31, 2021
Merged

Travis: line length tweaks #128

merged 2 commits into from
Jan 31, 2021

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 7, 2020

Proposed Changes

Use multi-line conditions to reduce line-length.

Note: this doesn't solve all the "warnings" and AFAICS, the other warnings can't really be solved anyhow as it would necessitate breaking up command to multiple lines which would break the command.

Related Issues

#123

Use multi-line conditions to reduce line-length.

Note: this doesn't solve all the "warnings" and AFAICS, the other warnings can't really be solved anyhow as it would necessitate breaking up command to multiple lines which would break the command.
@jrfnl jrfnl mentioned this pull request Dec 7, 2020
4 tasks
@Potherca
Copy link
Member

Potherca commented Dec 8, 2020

I've had to resolve the line-length limit in Yaml files before with commands like this.

As I recall it was possible to clean things like this up. I used to use https://yaml-multiline.info/ to get to grips with which magic incantations to use but it seems to be broken at the moment :-(

Anyway, I'll see if the remaining warnings can be resolved, otherwise I'll add a comment to exclude the offending lines.

@jrfnl
Copy link
Member Author

jrfnl commented Dec 8, 2020

@Potherca I'm still not convinced that breaking up commands to multi-line or adding exclude comments for a CI script to a CI script is really improving the readability of things or indeed, adding any QA value to this project.

@Potherca
Copy link
Member

Potherca commented Dec 8, 2020

I'm still debating myself over the value of things... One the one hand, there is getting the little things right before evolving into larger things. On the other hand there is not devolving into minutiae. I think I'll see what the fixed yaml looks like and see from there.

I'm with you in regards of readability... "Clean" but unreadable code (or config) is as bad hacky readable code...

@jrfnl
Copy link
Member Author

jrfnl commented Dec 8, 2020

@Potherca Well, the way things are going, we may need to move away from Travis altogether soon enough, so not sure it's worth spending much time on.

@Potherca
Copy link
Member

Potherca commented Dec 9, 2020

I'm in the process of migrating other projects to GitHub Action, I was planning on doing the same here... but the GitHub Actions config file is also Yaml, so the problem just moves about 😁

@Potherca
Copy link
Member

I've made some headway in migrating from Travis to GitHub actions. I am not done yet, so for now I think the best path is to silence the existing warning using # yamllint disable-line rule:line-length and merge this MR.

I'll fix/change things together with the work needed for migrating from Travis and documenting the CI/CD for contributors.

@jrfnl
Copy link
Member Author

jrfnl commented Dec 16, 2020

I've made some headway in migrating from Travis to GitHub actions.

Let me know if you have something you want me to look at. I've spend today familiarizing myself with it more and am slowly working my way through various repos which need migrating.

@Potherca
Copy link
Member

I'm keeping track of my findings of migrating from Travis to GitHub actions here: gist/9c05e2614cd1f96bffab6dee3bdb6360

@jrfnl
Copy link
Member Author

jrfnl commented Dec 17, 2020

@Potherca Nice! Reading through it now.

Possible additions which come to mind:

  • How to set up caching
    Interesting PHP/Composer specific action I found for this: https://github.com/marketplace/actions/install-composer-dependencies
  • Using continue-on-error for builds which would previously have been in "allowed_failures"
  • Options for "stages"
    • Using separate workflows with their own 'on' conditions
    • Or using separate jobs for stages which are conditional on the previous stage passing, in combination with using needs:

Other links with info I found useful (so far):

@Potherca
Copy link
Member

continue-on-error and cache were already on my list, I'll add "stages" too.

I've also done some investigation into making jobs depend on other jobs.

@jrfnl
Copy link
Member Author

jrfnl commented Dec 17, 2020

I've also done some investigation into making jobs depend on other jobs.

Nice! One I'm still struggling with is one where workflow A is run on push and pull_request and then workflow B should only run when A succeeded + only on push and workflow C should only run when A succeeded + only on pull_request.
If you have any thoughts on that ... ?

Complex builds are fun to convert ;-) (as if we didn't have anything better to do with our time)

@Potherca
Copy link
Member

Potherca commented Dec 18, 2020

As this is all getting rather involved and off-topic, should we move our conversation to a more sensible channel?

@mjrider And I hang out on Gitter and Discord and some other places... I've just created https://discord.gg/RerGp48hhC for discussing migrating from Travis to GHA

Is that doable for you or is there anywhere else in particular that comes to mind for you?

@jrfnl
Copy link
Member Author

jrfnl commented Dec 18, 2020

@mjrider And I hang out on Gitter and Discord and some other places... anywhere in particular that comes to mind for you?

I definitely don't use Discord (unless forced at knife point). I check Gitter when I'm pinged, but only then. Unfortunately have to be on Slack regularly, so yet another Slack would be easy enough to add. Other than that: Twitter DM ? video calls ?

@Potherca
Copy link
Member

I've got everything plugged into ferdi, so it doesn't matter that much to me... I personally prefer async and text-based, so I only do video for brainstorms and quick-alignment calls.

I'd rather keep my Twitter DM more for the general populace, I also don't respond there too quickly (like LinkedIn and email... often there's a 2 or 3-day delay).

So it would be a coin-flip between Slack and Gitter...

@mjrider
Copy link
Contributor

mjrider commented Dec 18, 2020

rather not slack, so gitter for me

@Potherca
Copy link
Member

Potherca commented Dec 18, 2020

Gitter it is then! Voila: https://gitter.im/Migrating-from-Travis-CI-to-GitHub-Actions/main

@Potherca Potherca merged commit 5ddc65b into master Jan 31, 2021
@Potherca Potherca deleted the feature/travis-tweaks branch January 31, 2021 12:14
@Potherca
Copy link
Member

I've added a commit resolving the 120-character limit violations, so other MRs can move forward without noise/failing builds.

Other/Further concerns should be resolved in subsequent improvements (and/or when migrating from Travis to GHA).

fi
- |
- >
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this correct ?

Copy link
Member

Choose a reason for hiding this comment

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

It'll work but I think you are right that it is not correct (i.e. could lead to trouble later). I'll add a fix before tagging and including it in the upcoming minor release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants