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

Automating regeneration of Info file #379

Open
jvdydev opened this issue Oct 2, 2023 · 13 comments
Open

Automating regeneration of Info file #379

jvdydev opened this issue Oct 2, 2023 · 13 comments
Labels
discussion Discuss things, make decisions enhancement New feature or request

Comments

@jvdydev
Copy link
Contributor

jvdydev commented Oct 2, 2023

From time to time, we get merge conflicts because two PRs change documentation, which results in both regenerating the info file (thus leading to a merge issue).
It also has happened that regeneration was forgotten.

Therefor, we want to move the building of the info file into CI.
I threw together a small workflow, however I first want to give anyone interested the chance to discuss (improvements, optimizations, ...).

The current workflow (branch set to craftedv2RC1, eventually this would become master):

name: regenerate-info

on:
  pull_request:
    types:
      - closed
    paths:
      - docs/*.org
    branches:
      - craftedv2RC1

jobs:
  regenerate-info:
    if: github.event.pull_request.merged == true
    runs-on: ubuntu-latest

    permissions:
      contents: write

    steps:
    - uses: actions/checkout@v3
      with:
        ref: ${{ github.head_ref }}

    - name: Install Emacs
      uses: purcell/setup-emacs@master
      with:
        version: 29.1

    - name: Generate info
      working-directory: ./docs
      run: make

    - uses: stefanzweifel/git-auto-commit-action@v4
      with:
        commit_message: Regenerate info file

This would rebuild the info file if any org file in the docs directory changed.
The newly built info file would then be committed by the last committer (co-authored by gihub-actions bot).

Discussion welcome 😄

@jvdydev jvdydev added enhancement New feature or request discussion Discuss things, make decisions labels Oct 2, 2023
@jeffbowman
Copy link
Contributor

Looks good to me!

@sthesing
Copy link
Contributor

sthesing commented Oct 6, 2023

Looks good to me, too! Thanks for digging into this.

@jvdydev
Copy link
Contributor Author

jvdydev commented Oct 10, 2023

So after I thought I had fixed the issue (from push to pull_request + a check if it's closed), apparently that is ran after the PR has been merged and pushes onto the "to-merge" branch.

Meaning the target branch doesn't get the updated info buffer, but the source branch of the merge does ... after the merge.

I'll look into this more and see if there is an "before merging" or something

@jvdydev
Copy link
Contributor Author

jvdydev commented Oct 10, 2023

After some more research, it seems there is no way to push to the target branch on PR.

We could simply run the action on the "main" branch (currently craftedv2RC1), there is only really PR merges going there.
Even if one of us pushes to main directly, that also would trigger the action which is probably what we would want in that situation anyway.

However, if someone forks, they have to create a new branch before committing (e.g. to make a PR), otherwise the Action will trigger for them.
Although arguably, people shouldn't push to craftedv2RC1/master on their fork either.

Thoughts?

@jeffbowman
Copy link
Contributor

jeffbowman commented Oct 10, 2023

However, if someone forks, they have to create a new branch before committing (e.g. to make a PR), otherwise the Action will trigger for them.

Umm... That's interesting. So forking this repo will get all the "extras" like this (and possibly linting in the future)... that's sorta unfortunate. I'd like to see that stuff here, but I'm not sure I want to inflict it on someone who forks the repo.

We could simply run the action on the "main" branch (currently craftedv2RC1), there is only really PR merges going there.
Even if one of us pushes to main directly, that also would trigger the action which is probably what we would want in that situation anyway.

This seems fine to me though and I'd support this.

Although arguably, people shouldn't push to craftedv2RC1/master on their fork either.

I'm not sure we have (or should have) that level of control on someone else's fork... I mean, if they just want to push on their fork, they can do what they want. We might need to check the settings on this project and make sure only maintainers can commit and/or merge PRs and avoid possible "oopsie!" from happening.

@jvdydev
Copy link
Contributor Author

jvdydev commented Oct 10, 2023

The last resort could also be to manually run the Action, however it somewhat defeats the point of having the action (except for ensuring a cleanly built info file, but the Makefile already does that) ...

People can remove the .github directory in their fork, would remove the actions (and bug templates etc.).
I guess they are passed down to forks as they are part of the git-versioned files.
Maybe there are some more conditional flags for running the Action, I'll look a little more into that.

@jvdydev
Copy link
Contributor Author

jvdydev commented Oct 10, 2023

Some more experimentation later, it seems I found the solution.
The following will run an action after the merge, build the docs, then push a commit to main with the updated info buffer (if the PR was merged and docs/*.org changed).

(I used main here, replace that in your head with whatever branch we want to run this on)

name: regenerate-info

on:
  pull_request:
    types:
      - closed
    paths:
      - docs/*.org
    branches:
      - main

jobs:
  regenerate-info:
    if: github.event.pull_request.merged == true
    runs-on: ubuntu-latest

    permissions:
      contents: write

    steps:
    - uses: actions/checkout@v4
      with:
        ref: ${{ github.head_ref }}

    - name: Install Emacs
      uses: purcell/setup-emacs@master
      with:
        version: 29.1

    - name: Generate info
      working-directory: ./docs
      run: make

    - uses: stefanzweifel/git-auto-commit-action@v5
      with:
        # This basically says "push to main"
        branch: main
        commit_message: Regenerate info buffer

Note that this does not work around the issue where this is still active when someone forks Crafted Emacs.

@jeffbowman
Copy link
Contributor

People can remove the .github directory in their fork, would remove the actions (and bug templates etc.).
I guess they are passed down to forks as they are part of the git-versioned files.

I guess that makes sense. I'd rather have the automation than worry about someone else having the same actions etc we have in their forks. We might mention that on the README somewhere under a section like "Forking this project" just so people are aware of the actions, etc that come with the project and how they might prefer to ignore / disable / remove those things.

@jvdydev
Copy link
Contributor Author

jvdydev commented Oct 10, 2023

Well, come to think of it, with the new version, it would only trigger an Action if the fork had a PR on itself to merge into the "main" of the fork (not on push) ... which seems unlikely (at least for now).

@jeffbowman
Copy link
Contributor

Yeah, that's a good point as well.

@sthesing
Copy link
Contributor

Much of the technical details of Github Actions are beyond me, so I'll keep out of that part of the discussion.

And I certainly don't want to stop you - @jvdydev - from tinkering with something that you enjoy. So, if you do enjoy it, knock yourself out.

If you don't, two thoughts:

  1. Do we need to keep the info file up to date in the repo? Couldn't we put it into .gitignore and call crafted-docs-export-info during startup?
  2. Maybe we need to take a step back and take a look at how big this problem actually is and how much effort it is worth. If I understand it correctly, we're trying to solve the following problem:
    • We require contributions that change the org docs to regenerate the info file. Because of that we sometimes get merging conflicts.
    • Resolving those conflicts is actually more hassle than just regenerating the info file after merging such a contribution, manually.
    • If we no longer require the regeneration of the info file, one of three things can happen:
      1. We have to manually regenerate it after the merge,
      2. We find a way to do it automatically inside the repo,
      3. We don't quickly regenerate the info file after the merge, which means the org-version of the docs will be up to date, the info-version will be out of date for a while.
  • I don't know how many users use info to read the docs, but even if it is the majority, we could put a general hint in the docs that the info file doesn't get regenerated with every commit and that the info docs might be slightly out of date, followed by instructions on how to update them locally.
  • If we maintainers regenerate it once in a while, I don't think the info file would be out of date long enough to cause much grievance. (Come to think of it, maybe we could run make docs in a local git hook).
  • We could add a bit of structure and predictability by introducing some form of semantic versioning after the official release of Crafted Emacs v2. Then, we could establish a rule that we regenerate the info file when we tag a minor version. That way, a user who is for example running some commit between v2.4 and v2.5 could know that the info docs would reflect the state of 2.4.

@jeffbowman
Copy link
Contributor

Here are my thoughts:

  • Automation is fun and can improve the quality of what we are doing. Regenerating the info file is one of those things where the quality of what we deliver can be improved by automating generation. Running a commit hook or a GitHub Action is approximately the same thing - the automation of generating the info file. I'm comfortable with either.
  • Conflicts happen, but I largely don't care about info file conflicts. I only worry about conflicts in the org file and regen the info file. If the regeneration step is automated, I just worry about resolving the conflicts and push.
  • Generating the info file on startup is not a bad idea, we can consider it. However, I don't start Emacs that often. My Emacs lives usually for several months. So, only regenerating the info file on startup won't work for me because I could pull changes many times before restarting Emacs which means my info file would be stale - and I'm one of those people who spends quite a bit of time reading info files.
  • Keeping everything reflecting the same information (ie, org files match the info file) in the repo, to me, "feels" like a higher quality product and is easier for the user to use once they clone/pull changes from the repo. Taking the opposite viewpoint, even if no one (or at least very few) reads the info file, this, to me, still "feels" better as the integrity of the solution is complete.
  • I really like the idea of versioning, we need to explore this idea and how/where we (possibly offline - or at least on a different thread than this particular issue):
    • keep the version number (in all the .el files, the readme, the docs, a "version" file, all of them?)
    • manage development (features on branches)
    • decide on next release processes (how/when to merge feature branches into a "release" and merge that to master for the next release and tag that point)

@jvdydev
Copy link
Contributor Author

jvdydev commented Oct 12, 2023

I mainly started this with the goal of having to "not think about" something that seemed like it can be automated (somewhat) easily.
I'm up for more discussion towards alternative solutions, it was just the only one I could think of (in terms of fully automating the process) at the time.

Generally, I think having the info file generated by the user on startup is not necessarily the solution as Emacs is not the only place to read info files (e.g. info command).
Also agreeing with Jeff regarding potential out-of-date of the info file in that scenario.

Regarding git hooks, that could work, I wonder if it would be a little slow (given there is not really any "incremental" building of the info file).

I like the idea of having versioning (e.g. through Milestones or similar).

@jvdydev jvdydev changed the title [v2] Automating regeneration of Info file Automating regeneration of Info file Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discuss things, make decisions enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants