chaoss / grimoirelab-elk

GNU General Public License v3.0
59 stars 120 forks source link

reset the counter after processed commits in branch #1058

Open shanchenqi opened 2 years ago

shanchenqi commented 2 years ago

Signed-off-by: Chenqi Shan chenqishan337@gmail.com

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2403637097


Files with Coverage Reduction New Missed Lines %
/home/runner/work/grimoirelab-elk/grimoirelab-elk/grimoire_elk/enriched/git.py 10 71.58%
<!-- Total: 10 -->
Totals Coverage Status
Change from base Build 2383854769: -0.006%
Covered Lines: 8828
Relevant Lines: 10722

💛 - Coveralls
sduenas commented 2 years ago

@shanchenqi I'm not familiar with this part of the code, what is it for?

shanchenqi commented 2 years ago

@shanchenqi I'm not familiar with this part of the code, what is it for?

Hi, @sduenas , Let me explain the bugs and my code. During the study enrich_git_branches , we reset the counter only after if commit_count == MAX_BULK_UPDATE_SIZE:. We should also reset the counter after if commit_count: by the same way. Otherwise, to_process list will be added to next for Loop when branched has been added by "if commit_count:".

jjmerchante commented 2 years ago

The problem here is that some commits from one branch, for example from development, will also be tagged with another branch name like master. The enriched items will be:

{
    ...
    branch: ["development", "master"]
}

while it should be:

{
    ...
    branch: ["development"]
}

This only happens when the enrich_git_branches study is enabled.

Merging this PR will fix the problem, but items enriched with the wrong branch won't be updated.

jjmerchante commented 2 years ago

@shanchenqi The PR looks ok but can we improve the commit message? We usually follow some guidelines: https://github.com/chaoss/grimoirelab/blob/master/CONTRIBUTING.md#guidelines-to-follow-to-write-good-commit-messages

We are also starting to include a changelog file with information about the PR for the release notes. You might need to install the package release-tools and run changelog and follow the instructions. We can do that step for you.

shanchenqi commented 2 years ago

@shanchenqi The PR looks ok but can we improve the commit message? We usually follow some guidelines: https://github.com/chaoss/grimoirelab/blob/master/CONTRIBUTING.md#guidelines-to-follow-to-write-good-commit-messages

We are also starting to include a changelog file with information about the PR for the release notes. You might need to install the package release-tools and run changelog and follow the instructions. We can do that step for you.

OK, @jjmerchante thanks for your help! I will follow the instructions in my next PR.

sduenas commented 2 years ago

Btw @shanchenqi , you don't need to create a new PR, you can just update this one ammending the commits and forcing an update of the branch.

vchrombie commented 2 years ago

@shanchenqi you need not create a new PR, you can add the changelog & amend the commit message, and later you can force push to your branch.

The following commands should help you

$ git add <path-to-your-changelog-entry>
$ git commit --amend # update your commit message here
$ git push -f origin git-enriched

Please let us know if you need any help. Thanks for your efforts.

shanchenqi commented 2 years ago

@shanchenqi you need not create a new PR, you can add the changelog & amend the commit message, and later you can force push to your branch.

The following commands should help you

$ git add <path-to-your-changelog-entry>
$ git commit --amend # update your commit message here
$ git push -f origin git-enriched

Please let us know if you need any help. Thanks for your efforts.

Excuse me, @vchrombie @jjmerchante there is still an error in my git commit changelog, could you help me transform it?

vchrombie commented 2 years ago

Excuse me, @vchrombie @jjmerchante there is still an error in my git commit changelog, could you help me transform it?

Hi @shanchenqi, I still don't see the changelog entry.

In case, if you are not familiar with the changelog entries, I'm adding a reference for you. https://github.com/chaoss/grimoirelab-perceval/blob/master/CONTRIBUTING.md#changelog-entries

You can use the interactive changelog tool for this purpose which generates the changelog entry file automatically.

Please go through it and reach out in case you need any help.

shanchenqi commented 2 years ago

@vchrombie Thank you! I added the changelog entry and all checks have passed.

sduenas commented 2 years ago

@shanchenqi the PR looks good but I edited the changelog file and the commit message to include it. I was trying to push a commit to your branch but I don't have access to it. I can provide you the change or you can give me access to that branch.

shanchenqi commented 2 years ago

Hi,@sduenas, thanks for your help. Maybe Git does not have branch specific permissions. Would you mind creating a pull request to my branch? After I merged it, this PR will be updated auto.

jjmerchante commented 2 years ago

Hi @shanchenqi, you can update the permissions from the right side of this pull request:

image

shanchenqi commented 2 years ago

Hi, @jjmerchante @sduenas I have already set the permissions for this pull request, could you try to push a commit to my branch again? image