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.

Sign up for GitHub

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

Jump to bottom

hedgedoc: 1.9.0 -> 1.9.4 #178129

Merged
merged 2 commits into from
Jul 21, 2022
Merged

hedgedoc: 1.9.0 -> 1.9.4 #178129

merged 2 commits into from
Jul 21, 2022

Conversation

drupol
Copy link
Contributor

@drupol drupol commented Jun 18, 2022 •
edited
Loading

Updated through the provided shell script.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@drupol drupol requested a review from Ma27 June 18, 2022 15:10
@drupol drupol marked this pull request as draft June 18, 2022 15:17
@ofborg ofborg bot requested review from globin and WilliButz June 18, 2022 17:21
@drupol drupol changed the title hedgedoc: 1.9.0 -> 1.9.3 hedgedoc: 1.9.0 -> 1.9.4 Jul 19, 2022
@drupol drupol marked this pull request as ready for review July 19, 2022 16:02
winterqt
winterqt requested changes Jul 19, 2022
View reviewed changes
Copy link
Member

@winterqt winterqt left a comment

Choose a reason for hiding this comment

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

Please edit the message of your first commit to follow the commit name guidelines, i.e. hedgedoc: add missing dependency to update script or similar.

Minion3665
Minion3665 suggested changes Jul 20, 2022
View reviewed changes
Copy link
Member

@Minion3665 Minion3665 left a comment

Choose a reason for hiding this comment

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

Unfortunately that commit message still doesn't quite work as it has a period at the end. Please read the commit message guidelines for more info

For consistency, there should not be a period at the end of the commit message's summary line (the first line of the commit message).

@drupol
Copy link
Contributor Author

drupol commented Jul 20, 2022

Fixed.

winterqt
winterqt requested changes Jul 20, 2022
View reviewed changes
Copy link
Member

@winterqt winterqt left a comment

Choose a reason for hiding this comment

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

Can you please drop the capital on the first word in the commit message (A -> a)? Thanks!

@drupol
Copy link
Contributor Author

drupol commented Jul 20, 2022

Fixed

@drupol
Copy link
Contributor Author

drupol commented Jul 20, 2022

@GrahamcOfBorg build hedgedoc

@SuperSandro2000 SuperSandro2000 merged commit a174de1 into NixOS:master Jul 21, 2022
@drupol drupol deleted the hedgedoc/bump-to-1.9.3 branch July 21, 2022 09:55
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jul 23, 2022 •
edited
Loading

This PR wasn't tested on a real system, right? The build instructions changed and the editor cannot be loaded after this update. Going to fix it myself.

Edit: Was looking at the develop branch which is the 2.0 version.

@drupol
Copy link
Contributor Author

drupol commented Jul 23, 2022

I locally tested the thing, everything seems right at first sight. Today when deploying in production. We noticed that there was an issue. I asked in the hedgedoc matrix and our build procedure is wrong. We have to use yarn instead of npm. I tried to do it, but I have the same issue locally. Help is very welcome at #182607

@winterqt
Copy link
Member

We have to use yarn instead of npm.

@drupol We are using yarn, so not sure what they meant by this.

@drupol
Copy link
Contributor Author

drupol commented Jul 23, 2022

We have to use yarn instead of npm.

@drupol We are using yarn, so not sure what they meant by this.

The build phase uses npm commands.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jul 23, 2022 •
edited
Loading

The build phase uses npm commands.

yarn build is an alias to yarn run build and both commands just run the build script. Also building the sqlite3 extension is also the same. Something is wrong with building hedgedoc itself or webpack.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jul 23, 2022 •
edited
Loading

Our hedgedoc includes this file which upstream does not and has a js error in it. Something with our vendoring is wrong.

webpack:///node_modules/js-yaml/dist/js-yaml-exposed.mjs

@SuperSandro2000
Copy link
Member

fixed in #182643

@mweinelt
Copy link
Member

mweinelt commented Sep 8, 2022 •
edited
Loading

Fixes an enumeration vulnerability in uploaded files in version 1.9.3 and should have been backported.

Especially commits should mention changelogs/release notes and reviewers should take an interest in quickly skimming them.

SuperSandro2000 pushed a commit to SuperSandro2000/nixpkgs that referenced this pull request Sep 8, 2022
@drupol @SuperSandro2000
hedgedoc: 1.9.0 -> 1.9.4 (NixOS#178129)
a0656e4
winterqt pushed a commit to winterqt/nixpkgs that referenced this pull request Oct 16, 2022
@drupol @winterqt
hedgedoc: 1.9.0 -> 1.9.4 (NixOS#178129)
3de2496 
(cherry picked from commit a174de1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Minion3665 Minion3665 Minion3665 requested changes

@winterqt winterqt winterqt requested changes

@Ma27 Ma27 Awaiting requested review from Ma27

@globin globin Awaiting requested review from globin

@WilliButz WilliButz Awaiting requested review from WilliButz

Assignees
No one assigned
Labels
1.severity: security 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@drupol @SuperSandro2000 @winterqt @mweinelt @Minion3665