deps: update to golang v1.21

Monirzadeh commented 9 months ago

try to update golang v1.14 to 1.21

[x] replace io/ioutil
[x] find all deprecated library
[x] update workflows

Monirzadeh commented 9 months ago

Hi @fmartingr i try to not change too much but please review this PR.

please check .golangci.yml do we need specific config?
reflect.SliceHeader and reflect.StringHeader deprecated i change function and the result is the same on my tests. do you know why we change string to byte that specific way?

fmartingr commented 9 months ago

Hi @fmartingr i try to not change too much but please review this PR.

please check .golangci.yml do we need specific config?

Not that I am aware of, I'd say let's use the same in all projects so results are the same.

reflect.SliceHeader and reflect.StringHeader deprecated i change function and the result is the same on my tests. do you know why we change string to byte that specific way?

I'm assuming they used it that way to avoid memory allocation as the comment suggest, never did something like that. We may need to add some benchmarks, but I wouldn't worry too much about that for now.

Now that we are on it, I think we can bump golang to 1.21 since 1.22 will release in February and 1.20 will become obsolete.

Monirzadeh commented 9 months ago

i review most of the workflows and remove what i think we don't need. currently keep golang-lint and add unit-test later when i add some unit-test for the project. change workflow golang-lint for new pull request. please run it manually to be sure everything work fine (if it possible).

side-note: we have some PR from dependencies not bad if you can review them. if you want i can fix them in this PR too

fmartingr commented 9 months ago

Why did you remove all linters and its configuration files from the workflows? You only left golangci-lint but removed both configuration files for it.

Maybe I expressed myself wrong, with my previous comment I meant to use the same yaml file for golangci-lint, but I think we should leave the other linters as well unless you have found they are not useful to this project. But they seem useful to me at a first glance (all but the shellcheck, since there does not seem to be shell files in the repo).

What do you think?

fmartingr commented 9 months ago

Also, update the dependabot.yml file with the same we have in shiori: https://github.com/go-shiori/shiori/blob/master/.github/dependabot.yml

This will make it group all updates in the same PR, which is easier to merge than a bunch of separate ones :)

Monirzadeh commented 9 months ago

I prefer the way we do golangci instead of being limited to the specific test, so I remove config of golangci. I remove spellcheck, docker and bash as you mention we don't have them in the project. Maybe I misunderstood this line.

I'd say let's use the same in all projects

So I try to keep things we have in shiori. I will return markdown and misspelling, but do you think we need action-alex?

fmartingr commented 9 months ago

Yeah, sorry, I was referring to golangci-lint specifically.

Please revert the changes for all linters but the shellcheck one, since I'm not aware of any bash/sh code in this repository.

Monirzadeh commented 9 months ago

@fmartingr I return alex misspell and super-linter for markdown. Remove spell-check and from super-linter two things docker and bash (we don't have docker file too). Update code-ql too.

Monirzadeh commented 9 months ago

I think it is ready now :+1:

Monirzadeh commented 8 months ago

Hey, left you some things around.

Also i would bring back the golangci.yml file since it has specific configuration that may be there for some reason, until we figure out what we need and what dont. Is there any speciifc linter or configuration causing false positives or something like that that I should be aware of?

in general i try to we have same settings in all shiori projects + specific needs for specific project as i know review that before default config for golangci-lint is here

Things we don't need in the old config and fix in the default config.

In the old config, structcheck' andvarcheck' are abandoned and we use unused instead in the default config.
in the old config rowserrcheck added to the old. that i find that not useful that is for sql (as i see we don't use sql in obelisk) you can check that here. in default config we don't have rowserrcheck

Exclude things in old config that we have in the default config.

gosec with config that exclude G108, I think it is better to find solution that exclude that if we have problem on that.
```
  - G108 # Profiling endpoint automatically exposed on /debug/pprof
```
what do you think? you can read more about that here
goconst that i think excluding this can't be a good idea you can read about that here but in general
```
Finds repeated strings that could be replaced by a constant.
```
dupl is the thing that maybe give us false positive we can return that if you want.

Specific settings in the old config that I think most of them are not necessary

errcheck that force this
```
# report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
# default is false: such cases aren't reported by default.
check-blank: true
```
I think it is not a big deal that we do other Shiori project.(if you want I can check the project for error handling in other PR too). should i return that?
govet with active check for shadowed variables (maybe useful do you want to active this option?)
we have gocyclo that increase min-complexity from 10 to 15 you can read about that here. i think this config just there because maybe some function is not good enough so i think lower variable (default 10) is better.
misspell that force to ignore-words: - someword do you think we need that? we have reviewdog/action-misspell too in workflow.

maybe we want keep spellcheck config. do we need that? you can read about that here

stylecheck:
go: "1.16"
# https://staticcheck.io/docs/options#checks
checks: [ "all", "-ST1003", "-ST1008", "-ST1016" ]
# https://staticcheck.io/docs/options#initialisms
initialisms: [ "ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS" ]

What do you think? based on this which part you want to keep? Sorry for the long comment, I try to cover all settings if I am not missing something.

fmartingr commented 8 months ago

Sorry for the long comment, I try to cover all settings if I am not missing something.

Don't be sorry, thanks a lot for your thorough investigation into this. Let's go with your changes since all points you mention seems fair to me. We can always change it later if we think it's necessary anyway. Awesome work!

go-shiori / obelisk