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

Continuous Release 1.9.0 #561

Merged
merged 18 commits into from
Jan 7, 2024
Merged

Continuous Release 1.9.0 #561

merged 18 commits into from
Jan 7, 2024

Conversation

Splines
Copy link
Member

@Splines Splines commented Nov 12, 2023

Warning

Ok, so I'm not sure why, but GitHub shows 4 commits up until "Add pgadmin..." that don't actually belong here. Instead, you can already find these commits on the main branch. This issue is exactly described here on Stack Overflow and has been viewed over more than 270k times. It seems like it is a limitation of GitHub. However, git log --oneline main..mampf-next also shows these commits in the diff and I'm not sure what we did wrong to end up in this weird situation.

1) I tried the rebasing solution in #579 and it was working perfectly fine and removed the 4 commits. The only downside of this approach is that due to the rebasing all the new commits of mampf-next would have the same timestamp: the time of performing the rebase. The will actually even get reassigned a new commit id.

2) If this first solution is not what we want, we could instead ignore the additional commits in this PR. The merge should work just fine. As we wanted to rename mampf-next to dev anyways, we could instead create this new branch dev from main after release 1.9.0 directly and ditch mampf-next. This way, we could be sure that main and dev match exactly and have the same commits on them.

No matter what option we choose, we will have a clean slate again starting with the next continuous release. Due to the nature of merge commits though, dev might be one commit behind main as it doesn't include the merge commit itself. See also this SO answer which explains this. That's not an issue though. One can avoid this by merging from main into dev directly after a continuous release such that dev can be fast-forwarded.

Changes

Also see #565 for a background on the linting changes.

For future releases?


  • check schema.rb has correct verison

Splines and others added 6 commits October 22, 2023 18:29
This is just to bring back a merge commit from `main` to `mampf-next`.
Will try to do a rebase from `mampf-next` onto `main` next time to avoid
these kind of PRs and merge commits.
* Prevent upload of empty files

* Add console log for empty file

* Add more specific error message for empty files

* Validate file size in backend

* Improve locale strings

* Undo automatic linting

* Undo unwanted changes

* Reset whitespaces

* Fix multiple files stats for bulk upload

* Provide info about which file is empty to user
* fix broken layout in submission table for lecture teacher or editors

* correct indentation
* Add pgadmin to dev docker compose

* Add pgadmin configuration

* Fix pgadmin config file docker binding

* Fix unwanted env comment
* Improve packet upload tooltip text

* Use "files" instead of "file"
Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Merging #561 (76cb7c0) into main (5b28b04) will increase coverage by 0.03%.
Report is 4 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #561      +/-   ##
==========================================
+ Coverage   66.49%   66.52%   +0.03%     
==========================================
  Files         311      311              
  Lines        9417     9426       +9     
==========================================
+ Hits         6262     6271       +9     
  Misses       3155     3155              
Files Coverage Δ
app/models/user.rb 53.43% <100.00%> (+0.42%) ⬆️
app/uploaders/correction_uploader.rb 100.00% <100.00%> (ø)
app/uploaders/submission_uploader.rb 100.00% <100.00%> (ø)

fosterfarrell9 and others added 7 commits November 12, 2023 16:48
* add trackable module to devise

* disable IP adress logging in trackable module of devise

* extend comment to explain better what happens

* remove redundant lines in comment
* Only lint Rubocop for now (and only modified files)

deleted files are excluded

* Scope action for PRs

* Disable tests for now

* Run command in one line

* Fix RuboCop command

* Further improve linting setup

* Simplify linter setup

* Print changed ruby files

* Fix further errors and improve workflow file

* Change random Rails files to see effects

* Fix spaces in changed files

* Include target branch lookup

* Remove unnecessary ${{}} syntax

* Fetch target branch

* Improve commit retrieval

* Redo whole workflow

* Revert dummy changes in Rails files

* Use checkout@v4 (instead of @V3)

* Explicitly set fail level & force excluding files

See command line flags here:
https://docs.rubocop.org/rubocop/usage/basic_usage.html#command-line-flags

* Add more events to trigger workflow

See the docs here:
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request

* Add printout to display RuboCop version used
* Shorten RuboCop config

Many cops were explicitly enabled. We don't necessarily need to do that
since the defaults of RuboCop are always included. So we just specify
where we deviate from RuboCops default values.

* Add comment regarding "NewCops"

* Add "NewCops: enable"

This has as consequence that whenever we upgrade RuboCop, we should run
checks on the whole codebase to see if new cops somehow don't work with
our code. We can then discuss if we maybe want to explicitly disable a new
cop.

* Update RuboCop related packages in Gemfile

* Add WordArray & EmptyMethod enforced styles

* Sort RuboCop keys alphabetically & improve file comment
* Let RuboCop autocorrect all Ruby files (safe)

Only safe autocorrections used. Command:
bundle exec rubocop --autocorrect --disable-uncorrectable

version: 1.57.2 (using Parser 3.2.2.4,
rubocop-ast 1.30.0, running on ruby 3.1.4) [x86_64-linux]
See the docs: https://docs.rubocop.org/rubocop/usage/auto_correct.html

704 files inspected, 8138 offenses detected, 7399 offenses corrected,
720 more offenses can be corrected with `rubocop -A`.

Note there occurred one error for the file "db/seeds.rb", which will
be fixed manually in subsequent commits.

* Revert "Let RuboCop autocorrect all Ruby files (safe)"

This reverts commit b067b2f.

* Force use of explicit hash literal value (HashSyntax)

* Only correct string literals to double quotes (safe)

704 files inspected, 3912 offenses detected, 3912 offenses corrected

* Allow use of method ".touch"

* Autocorrect all other specified cops (safe)

704 files inspected, 3171 offenses detected, 2432 offenses corrected,
720 more offenses can be corrected with `rubocop -A`

Command:
bundle exec rubocop --autocorrect --disable-uncorrectable

* Fix Layout/IndentationConsistency (manual)

* Fix (or disable) Layout/LineLength

* Fix all Style/ cops

* Fix Lint/ cops (3 still left open)

* Rename `get_votes_count` to `votes_count`

* Fix other Naming/ cops (1 still left open)

* Disable `Rails/HelperInstanceVariable` in `quizzes_helper.rb`

This is since we only use the instance variable to provide a *default*
value for some params, so the helper methods can still be reused.

* Fix all Rails/InverseOf cops

* Disable Rails/ cops in existing db migrations

The respective comments were added manually, not automatically.

* Fix Rails/SkipsModelValidations (1 left open)

Replaced `update_all` by `update`
and `Time.now` by `Time.current`

* Disable Rails/OutputSafety for one line

* Fix Rails/I18n related cops

* Add custom env variable to rubocop

This is to avoid Rails/UnknownEnv error

* Disable Performance/CollectionLiteralInLoop in some tests

* Merge two duplicates groups in Gemfile together

* Automatically autocorrect cops (unsafe)

Command used:
bundle exec rubocop --autocorrect-all --disable-uncorrectable

704 files inspected, 911 offenses detected,
909 offenses corrected

2 errors occurred:
An error occurred while Style/StringConcatenation cop was inspecting
mampf/app/models/medium.rb:871:30.
An error occurred while Layout/LineLength cop was inspecting
mampf/db/seeds.rb.

I manually fixed the error in medium.rb.
 I checked the seeds.rb and dit not find any error there.

* Fix missing `Time.zone.now`

* Add Style/MethodCallWithArgsParentheses and autofix (safe)

Command used: bundle exec rubocop --autocorrect --disable-uncorrectable

704 files inspected, 371 offenses detected, 350 offenses corrected,
13 more offenses can be corrected with `rubocop -A`

* Fix Layout/ cops

* Fix or disable Style/ cops

* Delete weird random character

* Fix Security/ cops

* Fix Performance/ cops

* Fix wrong namespace for a cop

* Manually ignore more db/ cop violations

* Delete empty test files

* Fix line to long

* Temporarily disable Rails/LexicallyScopedActionFilter

* Fix <= logical bug (registration threshold)

* remove duplicated restricted? method

* change private methods to non-private methods

* remove duplicate method

* fix typo

* change inverse_of relation

* fix existing inverse_of associations

* rename duplicate announcements method

* undo premature change

* add namespace to constant

* remove unnecessary logging

* remove non existing action from before filter

* Rewrite new registrations query with timeframe

* Allow touch_all method and replace problematic update calls

See this comment:
#566 (comment)

* Disable Rails/HasManyOrHasOneDependent rule

This is because we think it's cleaner not to have to write
"dependent: nil".

Also removed the respective rubocop:todo comments

* Fix unwanted "," character

* Get rid of weird "foo" check

* remove unused restricted? methods

* Replace problematic touch_all by touch

* Fix wrong manual Style/ correction of 5081aaa

* Fix Rails/OutputSafety

* Remove duplicate dates before save

see #566 (comment)

---------

Co-authored-by: fosterfarrell9 <[email protected]>
* Init new ESLint config to lint .js and .js.erb files

* Add default settings/extensions for VSCode

(maybe also something for another PR?)

* Add ESLint to CI/CD

* Introduce dummy changes to see ESLint in action

* Fix wrong fetch depth for checkout action

* Remove unnecessary "--" to pass arguments

* Outsource retrieval of changed files to reusable action

* Correct location of reusable action

* Fix further stuff related to GitHub action

* Clean up GitHub Actions files

* Cache global yarn cache directory

* Remove VSCode specific settings in favor of #569

* Revert "Introduce dummy changes to see ESLint in action"

This reverts commit cdf3473.
* Init default settings/extensions for VSCode

* Add further settings

* Improve "format on save" options

Also do not use deprecated flags in new VSCode version.

* Remove trailing comma in settings.json

* Enable ESLint as formatter
* Add cypress rules to ESLint & ignore some patterns

* Allow usage of tempusDominus global variable

* Ignore JS files with Sprocket syntax

* Further improve rules, e.g. allow common globals

* Ignore sprocket syntax in cable.js

* Autofix all `.js` and `.js.erb` files

Command used:
`yarn run eslint --fix .`

Still 47 problems (27 errors, 20 warnings) after this.

* Fix variables in turbolink fix

* Prepend unused variables with "_"

* Get rid of unused widget variable

* Fix specs comment tab alignment
@Splines Splines changed the title Continuous Release 1.8.3 Continuous Release 1.9.0 Jan 4, 2024
Splines and others added 4 commits January 4, 2024 22:44
* Upgrade ESLint configuration to new flat config

We sadly have to get rid of cypress for the moment.

* Run linter with `--no-warn-ignored` flag

See this answer: https://stackoverflow.com/a/77758970/9655481
for the background behind this change.

* Also lint the ESLint config file itself

* Get rid of `--ignore-path .gitignore`
* Replace background image

* Add slight transparency to boxes on landing page

* Recolor github badge (yellow)

* Add interactive aperiodic monotile to landing page

* Fix dragging of canvas

* Fix canvas dragging & z-index

* Add find out more button

* Add copyright notice for aperiodic monotiles

* Move to assets/javascript folder

* Fix turbolinks setup

* Make buttons less obtrusive

* Fix P5.js initialization

* Change to less obtrusive grey color

* Make colors darker & delete unused comment

* Adjust scaling and add black overlay

* Disable buttons according to their state

* Add touchMoved function

* Disable buttons on small devices

* Remove console log

* Make grey color lighter

* Fix JS layout (according to new ESLint rules)

* Add cypress rules to ESLint & ignore some patterns

* Allow usage of tempusDominus global variable

* Ignore JS files with Sprocket syntax

* Further improve rules, e.g. allow common globals

* Ignore sprocket syntax in cable.js

* Autofix all `.js` and `.js.erb` files

Command used:
`yarn run eslint --fix .`

Still 47 problems (27 errors, 20 warnings) after this.

* Fix variables in turbolink fix

* Prepend unused variables with "_"

* Get rid of unused widget variable

* Fix specs comment tab alignment

* Disable some ESLint rules for monotile code
* Upgrade Node.js to v20 (LTS)

see the release schedule here:
https://raw.githubusercontent.com/nodejs/Release/main/schedule.svg

* Use secure hash algorithm for webpack

Instead of the insecure MD4, we use SHA256.
The solution was proposed here:
https://stackoverflow.com/a/72219174/

* Refactor: extract variable

* Upgrade node version also for cypress tests

We do this even though we currently don't use this Dockerfile.
@Splines Splines changed the base branch from main to mails_tests January 5, 2024 23:37
@Splines Splines changed the base branch from mails_tests to main January 5, 2024 23:37
@Splines
Copy link
Member Author

Splines commented Jan 7, 2024

Currently, all existing model tests pass except for quiz_certificate_spec.rb. Already the first test there ("has a valid factory") fails with this error:

ActiveRecord::InverseOfAssociationNotFoundError:
Could not find the inverse association for quiz (:quiz_certificate in Medium) Did you mean?  quiz_certificates

You can verify that by your own on #581 which has all the changes from here merged into it.

As a reference, here is the "mantra" I once wrote for the inverse_of. Might be useful.

@Splines Splines marked this pull request as ready for review January 7, 2024 19:54
@Splines Splines requested a review from fosterfarrell9 January 7, 2024 19:54
@Splines
Copy link
Member Author

Splines commented Jan 7, 2024

For completeness: we've decided to go with option 2 of this comment. Also all tests were passing locally.

@Splines Splines merged commit 6ab42c6 into main Jan 7, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants