Skip to content
This repository has been archived by the owner on Feb 17, 2023. It is now read-only.

crontabs and common paths bundle #14

Merged
merged 6 commits into from Mar 1, 2012
Merged

crontabs and common paths bundle #14

merged 6 commits into from Mar 1, 2012

Conversation

neilhwatson
Copy link
Contributor

Crontabs is self explanatory. The paths bundle I'd like to be a community
effort to catalog the paths to commonly called commands.


files:

"${g.nhw_crontabs}/${user}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be modified to use paths.crontabs instead of g.nhw_crontabs.

@zzamboni
Copy link
Contributor

Hi Neil - thanks for the submission! I think the paths bundle is an excellent idea.

I have made a couple of comments on some lines.

@nickanderson
Copy link
Member

I like the paths idea too. Maybe its just me and my brain wanting to limit scope, does anyone else find it easier to read a commit that is just a single sketch?

@zzamboni
Copy link
Contributor

That's the idea in general - one sketch per pull request. In this case I think it made sense to bundle them because one uses the other.

--Diego

On Feb 28, 2012, at 7:39 PM, Nick Andersonreply@reply.github.com wrote:

I like the paths idea too. Maybe its just me and my brain wanting to limit scope, does anyone else find it easier to read a commit that is just a single sketch?


Reply to this email directly or view it on GitHub:
#14 (comment)

@neilhwatson
Copy link
Contributor Author

Made changes.

@nickanderson
Copy link
Member

On 02/28/2012 08:02 PM, Neil H Watson wrote:

Made changes.

Did you push your changes to your fork? I still see the g.nhw_crontabs
references in the pull request.

Nick Anderson nick@cmdln.org

@@ -0,0 +1,34 @@
bundle agent cronjobs(user, jobs) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this one perhaps be named cronjobs_add since it just appends if not found?

@nickanderson
Copy link
Member

neil what do you think about the bundle name change from cronjobs to cronjobs_add since it appends if not found. I think that would leave logical namespace for something like cronjobs_delete and other patterns for dealing with cronjobs, perhaps cron_d style related.

@nickanderson
Copy link
Member

I was thinking sketchname of cronjobs like you had but the only bundle you submitted in that sketch was also named cronjobs. I thought maybe just rename that bundle and we can add more cronjob related bundles to the sketch am curious what others think though before you keep making changes on my whims :)

@zzamboni
Copy link
Contributor

I agree - I think the sketch should be named cronjobs (or just cron), with the bundle names indicative of their function (e.g. cron_add, cron_delete, etc.)

@@ -0,0 +1,3 @@
author: Neil H Watson, neil@watson-wilson.ca
ostype: Any UNIX
Copy link
Member

Choose a reason for hiding this comment

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

Are these metadata values for ostype, tested, and cfengine_version supposed to be valid cfengine classes?
Which ones are required?

Should there be a comma between the name and email address?

I ask because I am thinking ahead to what the not yet finished tooling is going to want to expect and it would be nice to have a tool that could be used to validate a submission

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, ostype should be unix, which is a hard class defined by CFEngine on any Unix-style operating system. The author should preferably be Name <email@address.com>.

Indeed the idea is that we'll have a tool that can validate and submit sketches, and also one to install them.

Copy link
Member

Choose a reason for hiding this comment

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

Which if any fields are required?

@neilhwatson
Copy link
Contributor Author

I have one other large sketch to add. Is it better to wait for this pull to complete and then start a new one?

@nickanderson
Copy link
Member

Your in the same spot I was, your clone is progressing at a faster pace than upstream. I don't know what the official stance is, but it is probably best to get your stuff on a feature branch.
I found diasporas workflow to be pretty helpful, its concise at least.
https://github.com/diaspora/diaspora/wiki/Git-Workflow
Diego also pointed me to this which is a good read.
http://nvie.com/posts/a-successful-git-branching-model/

I see one more thing that needs to be addressed in the submission,
There is supposed to be a cf file matching the sketch name, so if you just rename system/cron/cronjobs_add.cf to system/cron/cron.cf and leave the bundlename as is. Then maybe diego can merge the pull request.

Then you should update your clone from upstream, and create a feature branch off of it where you can add your new sketch and submit a pull request for that.

I just had to work through doing that yesterday so if you need help let me know.

@zzamboni
Copy link
Contributor

zzamboni commented Mar 1, 2012

Hi Neil,

The best is to start a new branch for each sketch, then you can submit multiple pull requests simultaneously.

--Diego

On Feb 29, 2012, at 8:21 PM, Neil H Watson reply@reply.github.com wrote:

I have one other large sketch to add. Is it better to wait for this pull to complete and then start a new one?


Reply to this email directly or view it on GitHub:
#14 (comment)

@nickanderson
Copy link
Member

On 02/29/2012 08:57 PM, Diego Zamboni wrote:

Hi Neil,

The best is to start a new branch for each sketch, then you can
submit multiple pull requests simultaneously.

If you branch before your changes are merged and do a pull request on
that branch that pull request will include your other sketches.

Nick Anderson nick@cmdln.org

@nickanderson
Copy link
Member

I think these two sketches look good, anyone see any issues with merging?

@@ -0,0 +1,3 @@
author: Neil H Watson <neil@watson-wilson.ca>
ostype: Linux, Solaris 10, AIX
tested: Debian 6
Copy link
Member

Choose a reason for hiding this comment

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

oops, this should be a valid class name probably debian_6 ?

@zzamboni
Copy link
Contributor

zzamboni commented Mar 1, 2012

That's why you never develop on your master branch - that way every branch starts off with the "clean" state of the master branch, without any other changes, and they can be merged independently.

On Feb 29, 2012, at 8:58 PM, Nick Anderson wrote:

If you branch before your changes are merged and do a pull request on
that branch that pull request will include your other sketches.

zzamboni added a commit that referenced this pull request Mar 1, 2012
crontabs and common paths bundle.
@zzamboni zzamboni merged commit da6242c into cfengine:master Mar 1, 2012
tzz added a commit to tzz/design-center that referenced this pull request Jan 21, 2014
bundles.cf: rm_rf and rm_rf_depth bundles: add docs and port to 3.5
zzamboni added a commit to tzz/design-center that referenced this pull request Jun 1, 2014
tzz pushed a commit to tzz/design-center that referenced this pull request Jun 1, 2014
Added more content instructing how to write promises
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants