Skip to content

Allow disable automated cleaning#156

Merged
metal3-io-bot merged 4 commits into
metal3-io:release-0.4from
Nordix:feature/disk-cleaning-feruz
Jun 27, 2021
Merged

Allow disable automated cleaning#156
metal3-io-bot merged 4 commits into
metal3-io:release-0.4from
Nordix:feature/disk-cleaning-feruz

Conversation

@fmuyassarov
Copy link
Copy Markdown
Member

What this PR does / why we need it:
Implementation of disabling disk cleaning

@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 2, 2021
@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch 8 times, most recently from 9b115e2 to bd60363 Compare February 18, 2021 23:34
@fmuyassarov fmuyassarov requested review from kashifest and maelk and removed request for kashifest February 22, 2021 07:56
Copy link
Copy Markdown
Member

@namnx228 namnx228 left a comment

Choose a reason for hiding this comment

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

Overall, it looks good to me. Just one small question

Comment thread controllers/metal3machinetemplate_controller.go
Comment thread controllers/metal3machinetemplate_controller.go
Comment thread controllers/metal3machinetemplate_controller.go
@fmuyassarov
Copy link
Copy Markdown
Member Author

/retitle Allow disable automated cleaning

@metal3-io-bot metal3-io-bot changed the title ✨ Disabling disk cleaning implementation Allow disable automated cleaning Mar 8, 2021
@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch 2 times, most recently from 4faab8b to ea69883 Compare March 24, 2021 11:27
@metal3-io-bot metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 2, 2021
@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch from ea69883 to 90efdde Compare May 7, 2021 13:18
@metal3-io-bot metal3-io-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 7, 2021
@fmuyassarov fmuyassarov marked this pull request as ready for review May 7, 2021 13:20
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 7, 2021
@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch from 90efdde to cf30647 Compare May 10, 2021 13:46
@metal3-io-bot metal3-io-bot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 10, 2021
@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch 2 times, most recently from 6a6dff3 to 4e745d2 Compare May 23, 2021 19:35
@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch from f0e575b to 2a20df5 Compare June 24, 2021 07:28
@metal3-io-bot metal3-io-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 24, 2021
@fmuyassarov fmuyassarov changed the base branch from master to release-0.4 June 24, 2021 07:38
@fmuyassarov
Copy link
Copy Markdown
Member Author

/hold cancel
/test-integration

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 24, 2021
@fmuyassarov
Copy link
Copy Markdown
Member Author

/test-v1a4-integration

@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch from 2a20df5 to 974a7fb Compare June 24, 2021 20:23
@metal3-io-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fmuyassarov
Copy link
Copy Markdown
Member Author

/test-v1a4-integration

Copy link
Copy Markdown
Member

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

Mostly lgtm, some suggestions for improvement:

Comment thread baremetal/metalmachinetemplate_manager_test.go Outdated
Comment thread controllers/metal3machinetemplate_controller.go Outdated
Comment thread baremetal/metal3machinetemplate_manager_test.go Outdated
Comment thread baremetal/metal3machinetemplate_manager_test.go Outdated
Comment thread docs/api.md Outdated
Comment thread docs/api.md Outdated
Comment thread docs/api.md Outdated
@fmuyassarov fmuyassarov force-pushed the feature/disk-cleaning-feruz branch from 974a7fb to 16bb516 Compare June 25, 2021 16:32
@fmuyassarov
Copy link
Copy Markdown
Member Author

@furkatgofurov7 Thanks for the reviews. I've addressed all the comments, PTAL.

@fmuyassarov
Copy link
Copy Markdown
Member Author

/test-v1a4-integration

@furkatgofurov7
Copy link
Copy Markdown
Member

@fmuyassarov please squash your commits, I will lgtm after, thanks!

@fmuyassarov
Copy link
Copy Markdown
Member Author

@fmuyassarov please squash your commits, I will lgtm after, thanks!

I would like to keep commits separate to have clean history since each commit is doing different thing.

@furkatgofurov7
Copy link
Copy Markdown
Member

keep commits separate to have clean history

Ok, but I doubt that and IMO squashing the commits is the good way forward to maintain a clean git history.

Copy link
Copy Markdown
Member

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2021
@metal3-io-bot metal3-io-bot merged commit 125c953 into metal3-io:release-0.4 Jun 27, 2021
@fmuyassarov
Copy link
Copy Markdown
Member Author

fmuyassarov commented Jun 27, 2021

keep commits separate to have clean history

Ok, but I doubt that and IMO squashing the commits is the good way forward to maintain a clean git history.

I think it would be nice to have this documented somewhere, because I can see some PRs getting squashed and some not. Having formal instructions would help us to be consistent.

@fmuyassarov fmuyassarov deleted the feature/disk-cleaning-feruz branch August 14, 2021 19:40
honza pushed a commit to honza/cluster-api-provider-metal3 that referenced this pull request Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants