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

Fix typo #75

Closed
wants to merge 1 commit into from
Closed

Fix typo #75

wants to merge 1 commit into from

Conversation

kennethrrosen
Copy link

-set-management_dispvm-to-dvm-fedora was incorrectly replaced with -set-management_dispvm-to-default

-set-management_dispvm-to-dvm-fedora was incorrectly replaced with -set-management_dispvm-to-default
ben-grande added a commit that referenced this pull request Jun 24, 2024
@ben-grande
Copy link
Owner

Please see contribution instructions next time.

I didn't properly test this state after the change, sorry :(.

Anyway, the change is wrong, it is still referencing the state in qubes-builder.prefs, it should require the installation of the salt dependencies.


From CI:

--- Commit Message ----
Fix typo

-set-management_dispvm-to-dvm-fedora was incorrectly replaced with -set-management_dispvm-to-default
--- Meta info ---------
Author: kennethrrosen <[email protected]>
Date:   2024-06-24 10:33:36 -0400
is-merge-commit:  False
is-fixup-commit:  False
is-fixup-amend-commit: False
is-squash-commit: False
is-revert-commit: False
Parents: ['4b1b75a2408cc583b9c2162a93291908b835a488']
Branches: ['(HEAD detached at pull/75/merge)']
Changed Files: ['salt/qubes-builder/create.sls']
Changed Files Stats:
  salt/qubes-builder/create.sls: 1 additions, 1 deletions
-----------------------
1: CT1 Title does not follow ConventionalCommits.org format 'type(optional-scope): description': "Fix typo"
1: T8 Title is too short (8<10): "Fix typo"
3: B1 Line exceeds max length ([100](https://github.com/ben-grande/qusal/actions/runs/9647286265/job/26605832358?pr=75#step:9:101)>72): "-set-management_dispvm-to-dvm-fedora was incorrectly replaced with -set-management_dispvm-to-default"
DEBUG: gitlint.cli Exit Code = 3
Error: Process completed with exit code 3.

The commit Fix typo does not respect the gitlint policy that I have setup for the project. Please take a look at the history of commits to see how I do. Run gitlint locally.

The commit Fix typo also does not describe the issue, I have no idea which kind of typo, which kind of file, which formula. I don't have a "formula" on how to create commits, but please be more descriptive next time. See conventional commits for tips on how to write good commit messages.


I have fixed this issue in main to avoid other users facing this, read the contributing guidelines, lint locally first.

@kennethrrosen
Copy link
Author

Thanks, @ben-grande. I'll switch my workflow from working on github web to making any future contributions locally (for linting, etc.) and will read once more through the docs before any future PRs.

@ben-grande
Copy link
Owner

Use the dev formula. I know you used the Github web becasue it names branches as patch-N. This can get you lost. The dev formula has everything to contribute to qusal, this is what I use. It can also be used to contribute with QubesOS. I recommend running tests on disp-dev.

@kennethrrosen
Copy link
Author

That's a much appreciated tip. And I can use the dev formula without ancillary formulas like the git or split-* formulas?

@ben-grande
Copy link
Owner

You don't need the sys-git to use dev. You need the sys-pgp though to verify commit and tag signatures though, because I enforce it with merge.verifySignatures (git configuration), although you verification of pgp keys is done locally, when you need to sign, you will need the sys-pgp setup.

You will also need qusal.ConnectTCP, see sys-net/README.md to install the sys-net.install-proxy on the desired netvm, as dev does not have a netvm. If there are more questions, please open a new question issue for others to see it in the issue section instead of the PR section.

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.

2 participants