Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create mechanism for auto-pushing generated docs #397

Open
wyardley opened this issue Oct 18, 2017 · 25 comments
Open

Create mechanism for auto-pushing generated docs #397

wyardley opened this issue Oct 18, 2017 · 25 comments

Comments

@wyardley
Copy link
Contributor

@wyardley wyardley commented Oct 18, 2017

Lots of good discussion in #373

I'm going to close that PR, since it seems like things may be headed in a different direction; easy to bring back if we end up wanting to ignore the docs/ directory.

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Oct 21, 2017

One idea:

I've a pre_release rake task in mind which bumps the module version in metadata.json to a provided version and afterwards generates the changelog. We could here also regenerate the docs.

Benefits:

  • Simplifies release task
  • Doc updates are up2date for each release
  • Doc updates are reviewed before a release
  • Clean git history

Cons:

  • Docs aren't up2date after a regular pull request is merged. but that may not be needed
@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Oct 21, 2017

@voxpupuli/collaborators some feedback is welcome here :)

@dhollinger
Copy link
Member

@dhollinger dhollinger commented Oct 22, 2017

I like this idea as it makes sure that the new docs are generated with the metadata bump and changelog generation.

seems to me like the most logical place to put it at this time

@ekohl
Copy link
Member

@ekohl ekohl commented Oct 22, 2017

To confirm: when you release, the prepare_release:(patch|minor|major) task generates the docs + changelog and does the version bump. That's submitted in a PR. When you merge it, you create a tag and push it to Github which triggers Travis to push to the forge.

That would work for me.

Note that in the situation we don't bump the patch version after tagging but I'd be fine with that. You do need to ensure there's a prepare_release task that can skip the bumping but that's an implementation detail.

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Oct 22, 2017

Our travis_relase task checks if the version in metadata.json has no -rc0 and that a matching entry is in the CHANGELOG.md. Afterwards it creates a tag (which gets released by travis). Then it bump the version metadata.json by one patch version and appends -rc0.

@rnelson0
Copy link
Member

@rnelson0 rnelson0 commented Oct 30, 2017

Should this be part of travis_release, or added to blacksmith?

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Nov 2, 2017

@rnelson0 I've no objections on this.

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Nov 2, 2017

I currently hacked this in a modules Rakefile:

desc 'prepare a new release'
task :prepare_release, [:version] do |t, args|
  # check for missing parameter
  # exit early, exit often
  (puts 'you need to provide a version like: rake prepare_releasep[1.0.0]'; exit) unless args[:version]
  version = args[:version]
  (puts 'format needs to be X.X.X'; exit) unless /^\d+\.\d+\.\d+$/.match(version)
  ENV['BLACKSMITH_FULL_VERSION'] = version
  Rake::Task['module:bump:full'].invoke
  Rake::Task['changelog'].invoke
  Rake::Task['strings:generate'].invoke
end

This is ugly but works ¯_(ツ)_/¯. Anybody has objections here? Otherwise I would add git support to commit those changes.

@ekohl
Copy link
Member

@ekohl ekohl commented Nov 4, 2017

You have an additional p in rake prepare_releasep. I also have some more coding style thing where I prefer unless ; puts ; exit ; exit but in general I think that's fine.

@rnelson0
Copy link
Member

@rnelson0 rnelson0 commented Nov 8, 2017

I believe /^\d+\.\d+\.\d+$/ is a slightly better regex to ensure it doesn't match v1.0.0 or 1+0+0, but I'm not as good at Ruby regex to be sure from just looking at it.

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Nov 19, 2017

We got some nice recommendations for github releases on voxpupuli/voxpupuli-release-gem#17 which I will try to implement.

@alexjfisher
Copy link
Member

@alexjfisher alexjfisher commented Nov 19, 2017

I'm leaning to preferring not committing the autogenerated docs to github. But I don't honestly understand all the alternatives yet.

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Nov 19, 2017

@voxpupuli/collaborators some more feedback please :)

@dhollinger
Copy link
Member

@dhollinger dhollinger commented Nov 19, 2017

I'd prefer not committing the generated docs to github, however, I'm not sure Maintaining a separate branch for the github pages upload is that much better. Since the .pmtignore should keep the docs content out of the module build, I don't have a problem with it.

@rnelson0
Copy link
Member

@rnelson0 rnelson0 commented Nov 19, 2017

I think the rake task, in our release task gem, is a good start. We should not make it automatic until we test it out a bit, and that gives us time to find out if we want to make it automatic, too.

@wyardley
Copy link
Contributor Author

@wyardley wyardley commented Nov 19, 2017

For the way I tend to use docs, having the already generated docs locally is not that useful; I'd be much more likely to either look up docs online or read them via a pager or text editor than open them locally in a browser, whether I had connectivity or not.

I also don't like the extra history / diff items from committing generated docs.

Long-term, I do think a way to make useable plaintext docs (say, generating README.md) from strings is essential.

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Sep 2, 2018

Hi!
I played a bit with .gitattributes today. There are two nice options:

  • docs/** binary tells the git cli to not list diffs for anything in /docs/ in the repo (explanation)
  • docs/** linguist-generated=true disables the diff in the GitHub UI and excludes it from the language stats per repo (explanation 1 ,2).

I created a pull request as example at https://github.com/voxpupuli/puppet-selinux/pull/267/files

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Sep 2, 2018

@alexjfisher suggested that we generate a REFERENCE.md via puppet-strings instead. The forge can display it, one example is at https://forge.puppet.com/puppetlabs/ntp/reference

@alexjfisher
Copy link
Member

@alexjfisher alexjfisher commented Sep 3, 2018

Since it's just a single file, I'm hoping there'd be much less objection to it being committed to the repo.
@ekohl @dhollinger ?

Probably generate it at least in each release PR but maybe in-between releases as appropriate (eg during the commit where someone adds a new class or function, they'd also run the rake task). Since it's just 1 file and human readable, it's something I'd review. (Much like I review the changelog diff for anything that's obviously bad or confusing English etc.)

The forge directly supporting it also makes it a better choice to HTML files IMO.

@ekohl
Copy link
Member

@ekohl ekohl commented Sep 3, 2018

A single REFERENCE.md sounds much better. Since it's still a generated file, I do wonder when we'd update it though. Only on release? Or do we not store it in git at all - just the release?

@alexjfisher
Copy link
Member

@alexjfisher alexjfisher commented Sep 3, 2018

puppetlabs have theirs in git. Personally, I tend to read docs from github as it's normally soon after I have to take a look at the actual code! Also, if we don't store it in git we wouldn't have anything for reviewers to look at.

https://github.com/voxpupuli/puppet-extlib/blob/master/REFERENCE.md#debug-log-output is a right mess at the moment, but I'm glad that we can all see that.

@wyardley
Copy link
Contributor Author

@wyardley wyardley commented Sep 3, 2018

Assuming strings now supports building that, that’s exactly the type of feature I had wanted to have in the first place. I’m definitely +1 on this.

If it’s feasible to add to the tooling, I think the best thing would be to generate it on merges to trunk, and then commit it there are changes?

I think then that removing generated html docs from git would make sense, and saving that for either people to generate themselves and / or for online docs sites?

@bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Sep 5, 2018

I went ahead and created PRs for all modules that currently have a /docs/ in the repo:

I created the REFERENCE.md with bundle exec rake strings:generate\[',,,,false,true']

I didn't generate the file yet for:

./puppet-ferm/docs
./puppet-cassandra/docs
./puppet-mcollective/docs
./puppet-staging/docs
@ekohl
Copy link
Member

@ekohl ekohl commented Sep 5, 2018

I admit I also read the docs on github so I wouldn't mind including good docs. That means cleaning the output to be proper before we include the file in git. Often this means converting from rdoc to yardoc. I have some code to do that automatically, which I should finish. When a docs/ dir generated by puppet-strings is present in git then we should also remove it.

@wyardley
Copy link
Contributor Author

@wyardley wyardley commented Sep 6, 2018

Is it worth also revisiting the idea of generating the html docs to gh_pages branch (as well as having REFERENCE.md in the project)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.