-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implementation -- Brand new GitHub Action #37
Merged
SmolSir
merged 17 commits into
tonowak:implementation
from
mgr0dzicki:action_implementation
May 3, 2023
Merged
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
043f2f5
Describe action implementation
mgr0dzicki 9745df2
Fix section link
mgr0dzicki 348190b
Mention cache-related inputs
mgr0dzicki 0761e99
Apply suggestions from code review
mgr0dzicki 9ede3ad
Update thesis-en.tex
mgr0dzicki 4e9a3ad
Update thesis-en.tex
SmolSir 121e5a7
Update thesis-en.tex
SmolSir 598a548
Update thesis-en.tex
mgr0dzicki 2faafcd
Update thesis-en.tex
SmolSir c386f72
Update thesis-en.tex
mgr0dzicki 45e7ca4
Apply suggestions from code review
mgr0dzicki 59126d5
Update thesis-en.tex
tonowak 3ff5244
Update thesis-en.tex
tonowak 6ffe43b
Update thesis-en.tex
tonowak e4522b3
Update thesis-en.tex
mgr0dzicki f24efc7
Update thesis-en.tex
mgr0dzicki 96de66f
Update thesis-en.tex
tonowak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -709,7 +709,7 @@ \section{cargo-semver-checks-action}\label{r:section_cargo_semver_checks_action} | |||||
we implemented the V2 version of the action in a platform-independent way using TypeScript | ||||||
and Node.js, allowing it to be used on all runners supported by GitHub Actions. | ||||||
The differences between the V1 and V2 of the action are further discussed in section | ||||||
\ref{r:section_continuous_integration_improvements}. | ||||||
\ref{r:section_github_action}. | ||||||
|
||||||
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% | ||||||
% Implementation % | ||||||
|
@@ -727,7 +727,65 @@ \section{Command-line interface improvements}\label{r:section_cli} | |||||
|
||||||
\section{Brand new GitHub Action}\label{r:section_github_action} | ||||||
|
||||||
<Here write a list about the V2 of the continuous integration> | ||||||
The V1 version of cargo-semver-checks-action was created when | ||||||
cargo-semver-checks lacked a built-in logic of choosing the correct baseline version and was not | ||||||
generating the rustdoc output automatically. Both of these tasks where therefore handled by the action. | ||||||
When our work on the project started, the tool had already gained these features making the | ||||||
action obsolete. Its only advantage over the manual configuration was that it | ||||||
installed both the Rust toolchain and the tool itself. However, it downloaded the baseline version from | ||||||
the repository instead of using the latest release published on crates.io and did not handle the edge cases | ||||||
properly. The other problem was that the action did not allow passing any parameters to the tool, which | ||||||
was pointed out by the users who often decided to setup the tool manually for this reason. | ||||||
Finally, the action was implemented as a bash script, which made it difficult to maintain | ||||||
SmolSir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
and runnable only on Linux hosts. | ||||||
tonowak marked this conversation as resolved.
Show resolved
Hide resolved
tonowak marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
The need for a redesign was therefore clear. After reviewing the available options we decided to use the fact that a GitHub action might | ||||||
be run using Node.js and implement the V2 version in TypeScript. It is a standard way | ||||||
tonowak marked this conversation as resolved.
Show resolved
Hide resolved
mgr0dzicki marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
of creating complex actions, as it has multiple advantages: | ||||||
\begin{itemize} | ||||||
\item TypeScript action is platform-independent and can be run on all runners supported by GitHub Actions. | ||||||
\item Using Node.js makes it possible to depend on variety of \texttt{npm} packages, in particular the GitHub Actions | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
Toolkit \cite{github_actions_toolkit}, instead of building the components from scratch. | ||||||
\item TypeScript code is more readable and easier to organize than a bash script or a GitHub workflow file. | ||||||
\end{itemize} | ||||||
The new version of the action not only fixes the problems of the previous V1, but also | ||||||
adds numerous features: | ||||||
\begin{itemize} | ||||||
\item input \texttt{rust-toolchain}, which allows the user to specify the Rust toolchain to use instead | ||||||
of the default \texttt{stable}, | ||||||
\item inputs \texttt{package} and \texttt{exclude}, both accepting lists of package names, | ||||||
allowing to define the exact set of packages to be checked, | ||||||
\item input \texttt{manifest-path}, which allows to specify the path to the \texttt{Cargo.toml} file | ||||||
of the crate or workspaces to be processed if it is not located in the directory where | ||||||
the action is run, | ||||||
\item input \texttt{verbose}, which might be used to enable extended output of the tool, | ||||||
providing details about every executed lint, | ||||||
\item input \texttt{release-type}, with possible values \texttt{major}, \texttt{minor} and \texttt{patch}, | ||||||
allowing to specify the type of the release instead of deducing it from the crate's version change. | ||||||
Comment on lines
+755
to
+765
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't it be |
||||||
\end{itemize} | ||||||
Moreover, the action now includes several optimizations of the running time: | ||||||
\begin{itemize} | ||||||
\item The cargo-semver-checks tool is downloaded as a precompiled binary instead of being built from source. | ||||||
\item The rustdoc of the baseline version is cached between the action's runs, so that it is generated | ||||||
only once. The caching strategy might be adjusted by the user using inputs \texttt{shared-key} | ||||||
and \texttt{prefix-key} (e.g. to share the cache between different GitHub jobs). | ||||||
\item When possible, downloads from crates.io are made using recently developed "sparse" protocol | ||||||
tonowak marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
\cite{crates_io_sparse_protocol}. | ||||||
\end{itemize} | ||||||
tonowak marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
Along with the new version of the action, we also developed an extensive suite of action tests | ||||||
(not to be confused with the test suite of cargo-semver-checks). The current CI workflow | ||||||
of the project consists of over 30 checks, which are responsible for: | ||||||
\begin{itemize} | ||||||
\item making sure the action's code is properly formatted and builds without errors and warnings, | ||||||
\item running the unit tests, | ||||||
\item veryfying whether the action works on Linux, Windows and macOS with different versions of | ||||||
mgr0dzicki marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
Rust installed, | ||||||
\item testing all of the action's inputs by running it on a test repository, | ||||||
\item comparing the cache created by the action with the expected one. | ||||||
\end{itemize} | ||||||
Such a comprehensive collection of tests helped us to find and fix several bugs not only in the action, | ||||||
but also in cargo-semver-checks itself, and will make it easier to maintain the project in the future. | ||||||
|
||||||
\section{Test suite}\label{r:section_test_suite} | ||||||
|
||||||
|
@@ -1138,6 +1196,16 @@ \section{Rust community's positive response to the tool}\label{r:section_communi | |||||
% \url{/~https://github.com/obi1kenobi/cargo-semver-checks} | ||||||
|
||||||
|
||||||
%%% section 5.3 links | ||||||
|
||||||
\bibitem{github_actions_toolkit} GitHub, | ||||||
\textit{GitHub Actions Toolkit} (2023) \\ | ||||||
\url{/~https://github.com/actions/toolkit} | ||||||
|
||||||
\bibitem{crates_io_sparse_protocol} Inside Rust Blog, | ||||||
\textit{Help test Cargo's new index protocol} (2023) \\ | ||||||
\url{https://blog.rust-lang.org/inside-rust/2023/01/30/cargo-sparse-protocol.html} | ||||||
|
||||||
%%% section 7.2 links | ||||||
|
||||||
\bibitem{responsibilities-tomasz} GitHub, | ||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.