wert007 / commit-analyzer

Gets the time somebody worked on something from `git log`
MIT License
1 stars 1 forks source link

[Enhancement] Automated Source Code Optimisation #8

Open kevinmatthes opened 2 years ago

kevinmatthes commented 2 years ago

This issue originates from #6 and was created in order to mark it as an open task.

It is possible to automate the application of both cargo fmt and cargo clippy --fix to the source code. This issue is intended to discuss the options of realisation and the suggested implementation in further detail.

kevinmatthes commented 2 years ago

The basic options are:

All of them have their pros and cons. The minimal solution would be a Git alias whereas the Action would solve the problems automatically at the cost of a slightly higher maintenance effort.

wert007 commented 2 years ago

I actually don't really know all these options, which would you recommend and why? What are the differences between the approaches?

kevinmatthes commented 2 years ago

With the Git alias, you define a pseudo-command for Git to execute. Just like git add and git commit are Git commands, one could define git rust to automatically invoke the Rust optimisations and persist them in a new commit with a descriptive message. I have collected some of the most important in https://github.com/kevinmatthes/routines/blob/main/git-alias.m, if you want to take a look at further examples. The pros are that

The cons are that

With a pre-commit check, you define a shell script to check your code and save this script in the .git directory. If something is not optimised, you are not allowed to commit. I consider it rather painful to set it up, but it is an option after all, so I mentioned it. The pros are that

The cons are that

The GitHub Action is basically that what I suggested in #5. You just alter the file name, behaviour, required actions and permissions and so on, and it will enhance your commits automatically. The pros are that

The cons are that

wert007 commented 2 years ago

Regarding the GitHub Action, what do you mean by

  • you need to install action updates manually, and
  • you need to audit the actions you use before you use them.

exactly?

kevinmatthes commented 2 years ago

You need to pass a version information of the action you are about to apply. This might happen by naming the branch's identifier (namespace/action@branch), the most recent version's tag (namespace/action@tag001) or a commit hash (namespace/action@1234567890abcdef1234567890abcdef12345678).

Only with the latter method, you can be sure that you do not receive updates without any further notice. Something bad that might happen with your workflow is that it breaks after an automatic update. So for every new version, you need to lookup its commit hash to intentionally install exactly the new version you want. ("updating manually")

With "auditing" I mean that should take a look at the definition of the action to be sure that it does nothing in addition it should rather not do. You need to make sure that the action's behaviour is what you want and expect. This also correlates with the manual updates.

wert007 commented 2 years ago

I'm sorry, I still do not quite get it. I think I'll have to look at it myself some time. But I don't know when I'll have time for that.. 😅

kevinmatthes commented 2 years ago

The official GitHub Documentation on this topic, search for "Security Hardening", is a good starting point.

I would like to suggest to leave this point open for one of the next Pull Requests.

wert007 commented 2 years ago

Yeah like I said, I think I will dig into this myself. I will also set it up in the process of understanding everything about it. 😄

I leave this issue open, until I got to it.

kevinmatthes commented 2 years ago

When trying out this feature, remember to always set the permissions of the one-time repository token accordingly.

kevinmatthes commented 2 years ago

Another important point: when pushing to a GitHub repository with at least one GitHub Action configured, you need the workflow scope for your authentication token.

kevinmatthes commented 2 years ago

I found another option: a new sub-command for Cargo! This would be a standalone executable named cargo-something which performs the desired optimisations and might also commit, if possible, whenever one would call cargo something.

I intended something to be a placeholder and would like to suggest optimise to be a sane name for this command. What do you think about it, @wert007?

wert007 commented 2 years ago

Hmm, actually I think, that would have a few drawbacks, since e.g. I personally often use git commit -p or all my comment about git commit --amend would be harder to realise, if we actually expect people to not use git commit itself. And since both cargo fmt and cargo clippy are already subcommands of cargo, I do not think there needs to be one, that combines those two.

kevinmatthes commented 2 years ago

The auto-commit behaviour was just an optional suggestion. You could opt-in this feature, if required, for instance. A new commit would also originate from an optimisation GitHub Action so basically, the tool would automate or somehow abbreviate the following common workflow with the same effects as the GitHub Action:

cargo fmt
cargo c
cargo clippy --fix --allow-dirty --allow-staged

Committing or even just adding it to the staging area would become an optional service command depending on whether an according option is set. This is an advantage in contrast to the GitHub Action since you decide whether you commit or not whereas the GitHub Action would always commit.

kevinmatthes commented 2 years ago

Working prototype: https://github.com/kevinmatthes/cargo-optimise.

Install with cargo install --git https://github.com/kevinmatthes/cargo-optimise and run with cargo optimise. Yes, there is no hyphen necessary.

kevinmatthes commented 2 years ago

https://github.com/kevinmatthes/cargo-optimise: now less chatty, by default.

kevinmatthes commented 2 years ago

By the way: sysexits has a working example of GitHub Actions to at least lint and validate the format of the Rust source files. https://github.com/sorairolake/sysexits-rs/blob/develop/.github/workflows/CI.yaml

wert007 commented 2 years ago

Yeah, I really have to look into that. But I will probably first try it out in some test repository. 😅

kevinmatthes commented 2 years ago

If you already installed cargo-optimise, remove it with cargo uninstall cargo-optimise. The latest version does not work anymore with Cargo as intended. Hence, I renamed it to rs-optimise.