Skip to content
This repository has been archived by the owner on Dec 4, 2019. It is now read-only.

Fix a bug of updating Build#incremental_id #6

Merged
merged 1 commit into from Jul 24, 2013

Conversation

makimoto
Copy link
Contributor

Build#incremental_id should be 1 at first but nil actually now.
In this PR I fixed it.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling c6e4aac on makimoto:fix-build-id into cca28b2 on r7kamura:master.

@@ -75,7 +75,7 @@ def finish(result)
end

def update_incremental_id
update_attributes(incremental_id: job.builds_count)
update_attributes(incremental_id: job.builds.count)
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for your Pull-Request, but this change slows things down because it loses the benefit of counter cache.
I think what we should do for this problem are:

  1. Set a default value of jobs.build_count with 0 in a migration file
  2. Change incremental_id so that it starts with 1

Copy link
Owner

Choose a reason for hiding this comment

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

I'll fix this issue after merging your pull-request 👍

Copy link
Owner

Choose a reason for hiding this comment

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

Just fixed this, thx!

r7kamura added a commit that referenced this pull request Jul 24, 2013
Fix a bug of updating Build#incremental_id
@r7kamura r7kamura merged commit 2c32ef7 into r7kamura:master Jul 24, 2013
@makimoto makimoto deleted the fix-build-id branch July 24, 2013 13:54
@makimoto
Copy link
Contributor Author

🙇

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

Successfully merging this pull request may close these issues.

None yet

3 participants