search

TheContext / podcaster

Feed and website generator
Apache License 2.0
1 stars 0 forks source link

Rethink OutputWriter #11

Closed sockeqwe closed 5 years ago

sockeqwe commented 6 years ago

I would like to generate website out of the existing description, shownotes and people files which actually is yet another output (in addition to feed.rss and show notes).

At the moment this is done on OutputWriter. I try to understand what the purpose of Outputwriter in general is and what is the best way to add website generation. Should there be a FeedOutputWriter, ShownotesOutputWriter and WebsiteOutputWriter? Or should a single OutputWriter accept a list of "plugins" and then basically just do Single.merge() of all those plugins?

arturdryomov commented 6 years ago
sockeqwe commented 6 years ago

Just to clarify, Im not talking about generating whole website, but rather just a single site for a single episode (front matter format).

What do you think about the idea of replacing the outputwriter with something more generic like a Deployment. A deployment is basically just a

interface Deployment { fun deploy( p: Podcast, episodes : List) : Single

} We could have multiple "Deployments": - FeedAndShowNoteDeployment: basically uses OutputWriter to generate files and "deploys" that to ArtemZin/TheContext-Podcast git repository (via git). - WebsiteDeployment: Generates Front Matter Files for each episode which is basically description + show notes and "deploys" this files to TheContext/ TheContext.github.io git repository (which then generates static site via jekyll at https://thecontext.github.io/) Additionally ther could be other "deployments" too: - PreviewAtPullRequest Deployment: posts show notes preview back to the pull request that has added a shownote and description file so that it is easy to review. Then we could have a podcastci.jar that takes command line parameters which deployment should be active. For example on Master brach we run podcastci.jar --rssDeploymnet --websiteDeployment On Pull requests (for new episodes that we want to add) podcastci.jar --previewDeploments PS: Deployment is actually not the right word because what I'm describing with interface Deployment is actually "generate an artifact and deploy that one". open for better naming ... Artur Dryomov schrieb am Sa., 14. Juli 2018, 11:07: > > - I think the website supersedes show notes this way or another. > - Splitting the OutputWriter or not mostly depends on your needs. Do > you want to generate the website only? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > , > or mute the thread > > . >
sockeqwe commented 6 years ago

I will come up with a proposal later this week

arturdryomov commented 6 years ago

@sockeqwe, I saw you’ve started to rewrite everything, so I’ll give a couple of points 😉

From my point of view this tool should not be related to deployments at all. It creates some output from the defined input, that’s it. How and where the result is placed is totally situational and does not make a lot of sense if you think about someone else as a consumer of this thing.

I’m kind of against posting previews to PRs. I know it is kind of dream of yours but I don’t see a lot of benefit in it. I guess mostly because we haven’t defined the process, like, at all. So I’ve defined my own picture.

The PR procedure looks this way then.

  1. The change contains episode and people metadata.
  2. The local Git hook will regenerate Markdown files for the website and the RSS feed.
  3. Reviewing the PR in this scenario will allow to take a look at real changes to end products (Markdown and RSS).
  4. Merging the PR triggers Travis deployment which will regenerate static website and push RSS to a hosting.

This way we achieve separation of concerns and simplify the overall process. And far less possible things to break.

Does it make sense? I’m sorry to be a bit late to put this on paper.

sockeqwe commented 6 years ago

@sockeqwe, I saw you’ve started to rewrite everything, so I’ll give a couple of points 😉

Not everything, but yeah, I introduced some major changes.

Plain Markdown notes become obsolete. The website is used instead. RSS remains the same.

So we still have two artifacts and two deployment targets, right?

This tool is being run as a commit hook. Every change regenerates all the content on the current branch which is getting merged to master. This way we have a preview of what we get on the output and avoid using clunky schemes which might (and will) break eventually.

So you would like to have the following process:

  1. Hannes wants to publish a new episode 99. So he creates a new branch episode99, adds the .yml files to describe this episode and creates a pull request for it.
  2. Here starts the part that is not entirely clear to me: The commit hook gets triggered and generates the stuff (rss feed, shownotes, website, whatever ...) and adds this to the branch episode99?
  3. Artur reviews pull request (branch episode99) which contains .yml files (author Hannes) and generated stuff from commit hook?

Did I get this right? I think we discussed something like that at the very beginning to apply something like that to artem-zinnatullin/TheContext-Podcast but I think there was some reasons why we didn't like it (messes up git history, requires pulling branch and I can remember that @artem-zinnatullin had more a idiologic problem with this (that a CI doesn't do a deployment but rather changes diles directly).

My understanding of the desired process is:

  1. Hannes wants to publish a new episode 99. So he creates a new branch episode99, adds the .yml files to describe this episode and creates a pull request for it (in TheContext/episodes repo).
  2. CI starts and runs podcastci.jar --validation. So it only validates that the .yml files are correct (but doesn't deploy anything). If the yml files are not correct CI will fail the build and show this status in the pull request.
  3. If yml files are ok (checked by CI in step 2), Artur just reviews the .yml file content and approves the pull request
  4. Pull request accepted, branch episode99 gets merged to master.
  5. On every change on master we run podcastci.jar --deployRss --deployWebsite to regenerate all the stuff and "deploy" it to artem-zinnatullin/TheContext-Podcast (rss feed and plain show notes - ok obsolete could be removed) and website to TheContext/TheContext.github.io.
  6. END 😄

Using Travis will allow using secrets properly without hacks.

In my described process podcastci.jar runs on travis, so there is no hacking with secrets (unless we have a different understanding of what "hacking with secrets means")

And far less possible things to break.

Can't see a major difference where things would break in my described process comparing to your described process. Overall they seem pretty similar. only the deployment is handled differently.

Does it make sense?

Yes and no. My biggest problem is that basically a review of pull request for episode99 also review generated (and already unit tested) stuff. Also messing up git history is not my biggest problem, but having to git pull on every pull request request change from a reviewer (because new generated stuff has been added) to my local machine is a bit annoying.

I’m kind of against posting previews to PRs. I know it is kind of dream of yours but I don’t see a lot of benefit in it.

I would argue, that this simplifies Reviewers life quite a bit: If a reviewer sees a (markdown rendered) comment attached to the pull request for episode99, containing basically the show notes, you as a reviewer can simply focus on reviewing the actual text / content, not on reviewing .yml file's structure and formatting (which is already checked by podcastci.jar). Additionally, you can click on all links to validate if they open the desired websites which was the number one problem we had in the past imho.

arturdryomov commented 6 years ago

Not everything, but yeah, I introduced some major changes.

Yeah, it kinds of scares me, because you are reworking the whole thing in one change.

Did I get this right?

Seems right.

messes up git history

You do basically the same thing, just in a separate repository. And it does not mess it up, it just changes real files.

requires pulling branch

It doesn’t.

that a CI doesn't do a deployment but rather changes diles directly

That’s exactly what you do right now!

My understanding of the desired process is:

You’ve skipped the preview part. There is no way to check the sanity of RSS and Markdown in this scenario. Unless you introduce some complex tool that will spam PR with previews for each change.

Overall they seem pretty similar. only the deployment is handled differently.

Yep, but your changes paint a different picture 😉 You are incorporating deployments into the tool and I see it as a completely individual task that stands aside.

but having to git pull on every pull request request change from a reviewer

You’ll see generated changes in the PR itself since it will contain them. You do not have to pull them to see the change, that’s the whole point. The PR contains both source YAML and Markdown changes and generated RSS and Markdown for the website.

you as a reviewer can simply focus on reviewing the actual text / content, not on reviewing .yml file's structure and formatting

Yep, and I suggest doing exactly that without posting additional previews since it will be directly in the PR by design.

arturdryomov commented 6 years ago

I think the differences we have grow from the overall picture of how we see this thing going.

From my point of view this tool is not related to CI at all at this point. It is just a helper for a PR author allowing to generate files from centralized format instead of doing it manually (just like you do it right now).

I’ve tried to incorporate it in a potential CI workflow, but I always stumble upon over-automation. Yes, episodes can be in a one repository, the website in another one, it is quite possible to connect them and post previews of the generated stuff to the PR but is just not worth it for me. Launching it locally to generate necessary things and pushing this for a review makes much more sense and it folds nicely into the current workflow we have and doesn’t introduce any magic. The same thing, just more pleasant to use and without a fear of ruining the RSS feed by an accident.

sockeqwe commented 6 years ago

I know you are a smart guy. Thus, I completely trust your expertise. Lets do it the way you proposed. Also I'm not emotional attached to my code changes nor do I think my approach is better, just a different target picture. At the end all I care about is that it's automated somehow and I'm sure you have put a lot of thoughts in your proposed solution.

It's still not clear how website generation plays into the picture you have in mind. Should that be an entirely different project / jar? Do you have 10 minutes somewhen this week to talk about this?

I'm sorry if you feel that I reworked the whole thing and changed all your code. This was not my intention nor did I really do that. All I did regarding existing code is the following: https://github.com/TheContext/podcast-ci/pull/12/files

I changed Runner a bit to include website and the deployment thing. Your code is still to 95% there. I just renamed some classes and / or moved it to another package. I apologise if this caused any confusion or disappointment.

arturdryomov commented 5 years ago

Hopefully the initial goal of producing both website and feed is achieved.