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

changelogtest should be a separate standalone test ( not part of gitarro itself) #54

Closed
MalloZup opened this issue Sep 30, 2017 · 11 comments · Fixed by #101
Closed

changelogtest should be a separate standalone test ( not part of gitarro itself) #54

MalloZup opened this issue Sep 30, 2017 · 11 comments · Fixed by #101

Comments

@MalloZup
Copy link
Contributor

Figure out how to extrapolate the whole changelog flag and make a standalone test that gitarro will execute like

-t changelog.rb

@MalloZup MalloZup changed the title changelogtest should be a separate standalone test changelogtest should be a separate standalone test ( not part of gitarro itself) Sep 30, 2017
@MalloZup
Copy link
Contributor Author

MalloZup commented Oct 2, 2017

@juliogonzalez i think this --changelogtest option is kind of unrelated and is adding more complexity on the codebase.

The idea is to make a separate script that will be used on a job

@juliogonzalez
Copy link
Member

I agree.

We should maybe provide it inside the tools/ directory and then as an example.

@MalloZup
Copy link
Contributor Author

MalloZup commented Oct 6, 2017

@juliogonzalez afaik, the simplest solution for this, imho would be to create a docker container from gitarro, and freeze this container with the version of gitarro containing the changelog test.
( where we call changelogtest the whole container and would be just a unsupported tool, and we would have the container somewhere in CI but not here)

I had a look at the changelog tests and this is the simplest solution imho.

@juliogonzalez
Copy link
Member

Let me see how the changelog test works right now and think a little about it, before we take a final decission.

@MalloZup
Copy link
Contributor Author

MalloZup commented Oct 9, 2017

@juliogonzalez i have found a solution for this:

With this simply extrernal script:

´gitarro -t script´

git ls-tree --name-only -r HEAD^ | grep .changes .. etc

We can test if last commit changed the .changes file or not, which is what we want.

The only issue, is in case a PR doesn't need to modify the changelog.

@juliogonzalez
Copy link
Member

I am not 100% sure, but I think it won't work if the PR has several commits and the changelog was not changed at the last commit.

@juliogonzalez
Copy link
Member

I will have a look today, but IMHO we would need a simple ruby script to check GitHub API.

Since netrc will be already present, it should be easy to do it.

@MalloZup
Copy link
Contributor Author

MalloZup commented Oct 9, 2017

the only difficulty is that from gitarro to the test_script, the test script doesn't know which PR number is tested (gitarro know that), and other info.

If you use another script to use the github api, ( we will consume again rate limiting) :D .

good luck

@juliogonzalez
Copy link
Member

Then most probably we need to refactor the changelog test to make the currenct code easier :-D

@MalloZup
Copy link
Contributor Author

MalloZup commented Oct 9, 2017

atm the changelogtest is so refactored that it disappear :)

@juliogonzalez
Copy link
Member

Then maybe we will need to re-add it :-)

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

Successfully merging a pull request may close this issue.

2 participants