From 32a0a80777db7c1537f9f0f7c861c34c95ef58fa Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sat, 21 May 2016 14:04:55 -0400 Subject: review --- ...ent_1_b3343283b2d7d49ab70a95d762d0e081._comment | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 doc/todo/merge_request:_Propellor.Property.Sbuild/comment_1_b3343283b2d7d49ab70a95d762d0e081._comment (limited to 'doc/todo/merge_request:_Propellor.Property.Sbuild') diff --git a/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_1_b3343283b2d7d49ab70a95d762d0e081._comment b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_1_b3343283b2d7d49ab70a95d762d0e081._comment new file mode 100644 index 00000000..89583ffc --- /dev/null +++ b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_1_b3343283b2d7d49ab70a95d762d0e081._comment @@ -0,0 +1,25 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 1""" + date="2016-05-21T17:41:11Z" + content=""" +Re not running propellor in the sbuild chroot, I have in the past used +schroot for things where it would have made sense to run propellor +in the chroot. OTOH, systemd-container is a better fit for such uses cases +now, probably. + +Is the ~/.sbuildrc necessary to use the sbuild properties? If so, +would it make sense to have a property that configures it? + +You could use Utility.DataUnits for Ccache's MaxSize. This would be +more flexible and consistent with other things in propellor. + +Limit could be a monoid. This would perhaps simplify hasGroupCache +as it could only be used once to set multiple limits. + +Maybe instead of Ccache.hasGroupCache, call it Ccache.hasCache? + +That is a weird build warning! But, I don't see it with ghc 7.10.3. +Normally you'd see that warning when the module's export list exported the same +symbol twice. +"""]] -- cgit v1.3-2-g0d8e From fe80b0157c0de7c2a2155e5d268b965c3a0f443d Mon Sep 17 00:00:00 2001 From: spwhitton Date: Sun, 22 May 2016 01:48:28 +0000 Subject: Added a comment --- ...ent_2_d8afe7b1fd49df5794c9abf2be732f8b._comment | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 doc/todo/merge_request:_Propellor.Property.Sbuild/comment_2_d8afe7b1fd49df5794c9abf2be732f8b._comment (limited to 'doc/todo/merge_request:_Propellor.Property.Sbuild') diff --git a/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_2_d8afe7b1fd49df5794c9abf2be732f8b._comment b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_2_d8afe7b1fd49df5794c9abf2be732f8b._comment new file mode 100644 index 00000000..44a2a542 --- /dev/null +++ b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_2_d8afe7b1fd49df5794c9abf2be732f8b._comment @@ -0,0 +1,58 @@ +[[!comment format=mdwn + username="spwhitton" + subject="comment 2" + date="2016-05-22T01:48:27Z" + content=""" +Thanks for your feedback. + +> Re not running propellor in the sbuild chroot, I have in the past used +> schroot for things where it would have made sense to run propellor in +> the chroot. OTOH, systemd-container is a better fit for such uses +> cases now, probably. + +I was thinking that if someone wanted to use a schroot and run +propellor in it, useful properties could be appended to +`Propellor.Property.Schroot`. As far as types go, I think that the +types in `Propellor.Property.Chroot` would be sufficient. + +> Is the ~/.sbuildrc necessary to use the sbuild properties? If so, +> would it make sense to have a property that configures it? + +The only probably which *needs* the suggested ~/.sbuildrc is +`Sbuild.piupartsConfFor`. With the other properties and no +~/.sbuildrc, you should be able to go ahead and use sbuild(1) to +perform a clean build. + +I don't think there is a way to write a non-intrusive property to add +anything to a user's ~/.sbuildrc. That's because they will probably +have different preferences for the options to pass to piuparts than I +give in the example, and we would have to merge the adt-run code with +any existing post-build-commands. I'm not sure propellor should have +a perl config file parser. + +> You could use Utility.DataUnits for Ccache's MaxSize. This would be +> more flexible and consistent with other things in propellor. + +Done. + +> Limit could be a monoid. This would perhaps simplify hasGroupCache as +> it could only be used once to set multiple limits. + +Done. + +> Maybe instead of Ccache.hasGroupCache, call it Ccache.hasCache? + +Done, I think that's better. I was originally thinking that the name +`Ccache.hasCache` might be for a property `User -> Property +DebianLike`. However, if someone wanted to write a property configuring +a user cache, it would probably have the standard location +`~/.ccache`. This cache would be implicitly created when required, so +the name `Ccache.hasCache` would be needed. + +> That is a weird build warning! But, I don't see it with ghc +> 7.10.3. Normally you'd see that warning when the module's export list +> exported the same symbol twice. + +I'm on GHC 7.10.3, too... + +"""]] -- cgit v1.3-2-g0d8e From 65ac730c006184472a7d0cb19deffdd69839530f Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 22 May 2016 13:55:42 -0400 Subject: comment --- .../comment_3_679468488a88f0a3f28ea0be548691a0._comment | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 doc/todo/merge_request:_Propellor.Property.Sbuild/comment_3_679468488a88f0a3f28ea0be548691a0._comment (limited to 'doc/todo/merge_request:_Propellor.Property.Sbuild') diff --git a/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_3_679468488a88f0a3f28ea0be548691a0._comment b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_3_679468488a88f0a3f28ea0be548691a0._comment new file mode 100644 index 00000000..7d5da612 --- /dev/null +++ b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_3_679468488a88f0a3f28ea0be548691a0._comment @@ -0,0 +1,8 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 1""" + date="2016-05-22T17:53:42Z" + content=""" +Would it make sense to move the ~/.sbuildrc example into the documentation +for the property that uses it? +"""]] -- cgit v1.3-2-g0d8e From 4141e566dbf681ca3321009db462ada22657930c Mon Sep 17 00:00:00 2001 From: spwhitton Date: Sun, 22 May 2016 22:39:24 +0000 Subject: Added a comment --- .../comment_4_bae208f52cb01eeb6d95a06dd4d5466a._comment | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 doc/todo/merge_request:_Propellor.Property.Sbuild/comment_4_bae208f52cb01eeb6d95a06dd4d5466a._comment (limited to 'doc/todo/merge_request:_Propellor.Property.Sbuild') diff --git a/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_4_bae208f52cb01eeb6d95a06dd4d5466a._comment b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_4_bae208f52cb01eeb6d95a06dd4d5466a._comment new file mode 100644 index 00000000..fc7a8005 --- /dev/null +++ b/doc/todo/merge_request:_Propellor.Property.Sbuild/comment_4_bae208f52cb01eeb6d95a06dd4d5466a._comment @@ -0,0 +1,9 @@ +[[!comment format=mdwn + username="spwhitton" + subject="comment 4" + date="2016-05-22T22:39:24Z" + content=""" +I've copied the relevant part to the documentation for that property. + +I'd like to retain the whole suggested ~/.sbuildrc content at the top of the haddock. The purpose of the suggested config.hs lines is to set up everything you need to be able to run sbuild with both piuparts and adt-run -- the \"complete setup\". You don't get all of that by just running `sbuild-createchroot` from a command line. So having both the config.hs lines and the ~/.sbuildrc lines at the top of the haddock makes it clear what the module can do for the user. +"""]] -- cgit v1.3-2-g0d8e