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

Improve Travis Configuration #999

Merged
merged 1 commit into from
Jul 3, 2019
Merged

Conversation

rsalevsky
Copy link

  • switch to Xenial (Trusty is EOL soon)
  • sudo is now allowed by default
  • add Gitlint check

- switch to Xenial (Trusty is EOL soon)
- sudo is now allowed by default
- add Gitlint check
@rsalevsky rsalevsky requested a review from a user June 27, 2019 08:27
@ghost
Copy link

ghost commented Jun 27, 2019

Basically looks good to me, thanks.

I am wondering if we should just put the gitlint thing in our openSUSE container that also validates the documentation -- SOC is certainly not the only SUSE documentation project with dodgy commit messages and this appears to be packaged as python3-gitlint.

A few questions:

  • It seems to me that the ini file you are currently using requires a SOC- or bsc# reference in every single commit, which seems a bit inflexible--or am I reading that wrong?
    The issue I see with that is that people might start using bogus bugs like bsc#000 to get around such a requirement and that certainly does not make commit messages more correct or easier to read.

  • Squash merges are allowed in this repo [1] which means commit messages may not be the same on the feature branch that they are going to be after merging back to master. How does gitlint deal with that?

[1] I think they're evil but they have a way of re-enabling themselves a few weeks after I disable them in a repo.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good.

@rsalevsky
Copy link
Author

Basically looks good to me, thanks.

I am wondering if we should just put the gitlint thing in our openSUSE container that also validates the documentation -- SOC is certainly not the only SUSE documentation project with dodgy commit messages and this appears to be packaged as python3-gitlint.

Some months ago the package wasn't really useful (some requirements where not there). We use the pip version in our travis runs.

A few questions:

  • It seems to me that the ini file you are currently using requires a SOC- or bsc# reference in every single commit, which seems a bit inflexible--or am I reading that wrong?

Yes it does. Nevertheless as we extend the usage to more repos this might change, see SUSE-Cloud/automation#3509. The current idea is to allow trivial changes without referencing a bsc or SOC. Feedback is welcome in the automation PR.

The issue I see with that is that people might start using bogus bugs like bsc#000 to get around such a requirement and that certainly does not make commit messages more correct or easier to read.

Agreed and we should avoid that this happens. If it happens then I see the current config as counter productive.

  • Squash merges are allowed in this repo [1] which means commit messages may not be the same on the feature branch that they are going to be after merging back to master. How does gitlint deal with that?

You can configure it so that these commit are ignored. So far we never had this case.

[1] I think they're evil but they have a way of re-enabling themselves a few weeks after I disable them in a repo.

@rsalevsky
Copy link
Author

We changed the config file now so that not always a reference has to be added.

@ghost ghost requested review from kallecarlwa, dmpop and a user July 3, 2019 15:02
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Ok, looks good to me, then!

(I'll think about whether to include this in more doc repos separately, don't want to send stop energy here.)

[I have added the two thirds of the actual SOC writers that are currently not on vacation for reviews.]

@rsalevsky rsalevsky merged commit 43bd257 into SUSE-Cloud:master Jul 3, 2019
@rsalevsky rsalevsky deleted the travis branch July 3, 2019 15:49
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