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

Add new workflow to automatically (and periodically) update ex_doc version #65

Merged
merged 16 commits into from Oct 14, 2023

Conversation

MarkoMin
Copy link
Contributor

@MarkoMin MarkoMin commented Sep 5, 2023

Tried everything locally (except actually creating an issue). If it works correctly, it will create one at midnight. First time having fun with GH actions so there may be some mistakes.

Closes #57

@paulo-ferraz-oliveira
Copy link
Collaborator

@MarkoMin, I created an Elixir script your pull request could plug in to (if it gets merged first), so that a pull request also gets created alongside the issue. 👍

My pull request is mentioned above.

And here's an example of an automated pull request: https://github.com/paulo-ferraz-oliveira/rebar3_checkshell/blob/main/.github/workflows/rebar3_depup.sh (I do prefer to have my shell script separate so I can "shellcheck" and "shfmt" it 😄).

@MarkoMin
Copy link
Contributor Author

Oh, I didn't even think about that, that could work nicely! I'll utilize it if/when it gets merged.

@MarkoMin
Copy link
Contributor Author

One thought: do we actually need both an issue and a PR? We should switch to PR only IMO.

@paulo-ferraz-oliveira
Copy link
Collaborator

I thought about that, too. Maybe PR alone is enough.

@MarkoMin
Copy link
Contributor Author

Seems like it should be. But now we wait for #67 to get merged

@paulo-ferraz-oliveira
Copy link
Collaborator

@MarkoMin, #67's merged. It ended up being a Mix task. I think you'd need to compile it and run it, unless @starbelly has a different suggestion.

@paulo-ferraz-oliveira paulo-ferraz-oliveira changed the title ex_doc_version_check workflow Add new workflow to automatically (and periodically) update ex_doc version Sep 29, 2023
@starbelly
Copy link
Owner

A thought on this, and this may or may not be pragmatic, but we have both erlang and elixir at our disposal. We could write a bit of code that uses httpc to get this information and handle the result with more exactness (i.e., use verl or Version, Version if it's a mix task).

Other thought, since we're wanting to get the latest release that is available on hex, we can just hit the hex api to get this info.

$ latest_stable=$(curl -s https://hex.pm/api/packages/ex_doc | jq '.latest_stable_version' | tr -d '"')
$ echo $latest_stable
0.30.6

@paulo-ferraz-oliveira
Copy link
Collaborator

Good stuff. And jq has builtin support to remove the " too, so you can do jq -r '.latest_stable_version'. I'll suggest this above, instead of what I did before. I didn't quite get the Erlang idea, but feel free to expand. (e.g. you'd want the mix task to have no inputs and figure out the latest by itself?)

@starbelly
Copy link
Owner

Good stuff. And jq has builtin support to remove the " too, so you can do jq -r '.latest_stable_version'. I'll suggest this above, instead of what I did before. I didn't quite get the Erlang idea, but feel free to expand. (e.g. you'd want the mix task to have no inputs and figure out the latest by itself?)

Yeah. More like you can use erlang or elixir and httpc to do the gets and posts vs a bunch of shell scripterty, but that said. Let’s be pragmatic.

@MarkoMin
Copy link
Contributor Author

MarkoMin commented Oct 2, 2023

I've made following changes:

  1. Moved task from root to lib/mix/tasks
  2. Renamed workflow as per @paulo-ferraz-oliveira suggestion
  3. Update bash script to make an automated PR as per @paulo-ferraz-oliveira example - take a look if there are any commands that are redundant or missing

@paulo-ferraz-oliveira
Copy link
Collaborator

paulo-ferraz-oliveira commented Oct 2, 2023

I approved the workflows to run. I'm forking your branch to test out the flow...

@paulo-ferraz-oliveira
Copy link
Collaborator

paulo-ferraz-oliveira commented Oct 2, 2023

@starbelly, if this isn't done already we're gonna need to enable

image

and also

image

@paulo-ferraz-oliveira
Copy link
Collaborator

@MarkoMin, you copied to "lib/mix/tasks", instead of moving...

@paulo-ferraz-oliveira
Copy link
Collaborator

Example of a pull request working: https://github.com/paulo-ferraz-oliveira/rebar3_ex_doc_mm/pull/2/files.

@paulo-ferraz-oliveira
Copy link
Collaborator

@MarkoMin, sorry I did so many small changes/suggestions, but it was the best way I found to explain my reasoning. Accepting them all should put the pull request in a mergeable state.

@starbelly
Copy link
Owner

@starbelly, if this isn't done already we're gonna need to enable
image

and also
image

I think with how infrequent ex_doc releases are (and should be) we should just allow the bot to open PR. We definitely want to see what the bot has done before merging, thoughts?

@paulo-ferraz-oliveira
Copy link
Collaborator

That's my goal. I was under the impression you needed:

  • the latter to allow to write to branch
  • the former to open the pull request

But maybe the former is not required (?)

@paulo-ferraz-oliveira
Copy link
Collaborator

We can also accept+merge (once all changes are done), then test the settings that are required (our goal is only to create the pull request, as automating stuff like this can lead to issues - let's say that ex_doc breaks and we don't have enough tests, for example).

@MarkoMin
Copy link
Contributor Author

MarkoMin commented Oct 3, 2023

I approved all suggestions. Sorry for errors with mix, first time using it.

Now it should be done. It's quite messy with all those commits, but we can squash them I guess. For some reason, mix.exs got into commits, but changed version is the file that is currently on main so no conflicts should exists.


File.write!(@mix_exs, "#{content_up}\n")
env = [{"PATH", System.get_env("PATH")}]
System.cmd("mix", ["deps.unlock", "ex_doc"], env: env)
Copy link
Owner

Choose a reason for hiding this comment

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

We can do this and it should be fine, but probably more pragmatic is to run this, then execute mix deps.get back in the shell (i.e., in whatever workflow that will use this).

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't mind doing it in another pull request @starbelly, even before we release. This fix is already scope creep caused by me, since it wasn't in the change proposed by @MarkoMin initially.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MarkoMin, if you prefer to do it, you can remove these calls to mix from here and set them in the CI bit... But lemme know, otherwise I can also do in a subsequent pr (since this one's working as-is)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do it. Do I need to remove File:write also or just lines 14 and 15?

Copy link
Collaborator

Choose a reason for hiding this comment

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

14-16, then add their equivalent to .yml. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@paulo-ferraz-oliveira
Copy link
Collaborator

@MarkoMin, could you rebase this onto the main branch, and move the script?

I see this in the Files' tab

image

and it's confusing. Thanks.

MarkoMin and others added 15 commits October 4, 2023 11:22
Also, query for getting issues is fixed because by default it retrieves only open issues.
Co-authored-by: Paulo F. Oliveira <paulo.ferraz.oliveira@gmail.com>
Co-authored-by: Paulo F. Oliveira <paulo.ferraz.oliveira@gmail.com>
Co-authored-by: Paulo F. Oliveira <paulo.ferraz.oliveira@gmail.com>
Co-authored-by: Paulo F. Oliveira <paulo.ferraz.oliveira@gmail.com>
Also, query for getting issues is fixed because by default it retrieves only open issues.
Co-authored-by: Paulo F. Oliveira <paulo.ferraz.oliveira@gmail.com>
Co-authored-by: Paulo F. Oliveira <paulo.ferraz.oliveira@gmail.com>
@MarkoMin
Copy link
Contributor Author

MarkoMin commented Oct 4, 2023

@MarkoMin, could you rebase this onto the main branch, and move the script?

done

Copy link
Owner

@starbelly starbelly left a comment

Choose a reason for hiding this comment

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

LGTM! If it fails, we can fix it 😄

@starbelly starbelly merged commit c6cad2b into starbelly:main Oct 14, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trigger issue when new ex_doc releases have been released.
3 participants