openSUSE / gitarro

run all your test against a GitHub Pull request
https://opensuse.github.io/gitarro
MIT License
15 stars 20 forks source link

Use PRs #45

Closed juliogonzalez closed 7 years ago

juliogonzalez commented 7 years ago

@MalloZup, can you please make a single PR for the refactor and transformation into a gem?

I am trying to add a new functionality, but I see now duplicated code, and don't know where to make the changes anymore.

MalloZup commented 7 years ago

@juliogonzalez i will rebase it, and squash commits :)

juliogonzalez commented 7 years ago

Great, but don't do it on master!

What's already on master, should stay, or there will be conflicts for people who already cloned the repo.

Why we should be always develop and test with PRs and never send commits directly to openSUSE/gitarro.

MalloZup commented 7 years ago

@juliogonzalez yop, i took the only exception for this renaming , consider it as fixed, sorry for this inconvenients, the gitbot -> gitarro took some time :)

juliogonzalez commented 7 years ago

I still see duplicated code, one at lib/ the other at gem/lib (for example), so when I try to rebase it is impossible to do it :-(

If the only valid code is under gem/ then we need git to make aware that we are moving things from / to gem/

MalloZup commented 7 years ago

everything in gem directory is not valid, (i updated right now the doc).

I will add a gitgnore now for gem dir

juliogonzalez commented 7 years ago

@MalloZup I have conflicts at my local master fetch from upstream (this repo)

Did you rebased master at this repo?

juliogonzalez commented 7 years ago

Basically I can see the following commits at one of my FIX branches with commits that are not at master anymore:

@MalloZup MalloZup Remove gitbot from classname 7eb82d5 @MalloZup MalloZup Replace gitbot name class with simply backend cacf8fe @MalloZup MalloZup Remove gitbot dependencies in libraries de9b46f @MalloZup MalloZup Remove gitbot name in code 93fc2c3

MalloZup commented 7 years ago

@juliogonzalez yop i squashed this commits. Since this were part of the rubygem feature

juliogonzalez commented 7 years ago

@MalloZup never squash branches at master, because now the PR I have, an another one I was preparing need to be closed and I will need to prepare manual diffs since my branches do not share a common history with master anymore :-(

MalloZup commented 7 years ago

@juliogonzalez ok , sorry . Consider this as an exception that occurred because all this renaming stuff

juliogonzalez commented 7 years ago

No problem :-)

Just keep in mind that changing history at upstream (specially master) must never be done (no even as an exception), and even if the history is dirty

That's why we should use always PRs (or when working on repositories without other people, at least using branches and making clear to the people which branches are master or master-equivalent and which branches can change history).

MalloZup commented 7 years ago

@juliogonzalez ok thx for clarifications, normally i'm the PR fanatic too.

I think we should consider issue #42 asap.

This include the dockerimages, but what is more critical is that all our jenkins job are using the old name

gitbot

At moment we have a salt state that take the latest version of gitbot (cloned it) etc,

This means that if we dont want to apply the highstate 'pr-automaiton', :)

tomorrow i will fix the jobs via gui :cry: ( this is why we need a jenkinsfile or something)

ciao cu tm

MalloZup commented 7 years ago

i close this

juliogonzalez commented 7 years ago

You can also fix the jobs via API with a bash script. Well have a look tomorrow.