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

Rename DevFile to Devfile everywhere in the code. #18

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

thepetk
Copy link
Contributor

@thepetk thepetk commented Aug 8, 2023

What does this PR do?

This PR ensures that every variable having the name devfile does not contain a capital F for the devfile word. The only two exceptions are the func SelectDevFileFromTypes and func SelectDevfilesFromTypes which remain the same. A deprecation warning has been added in the code. Soon they will have to be removed in favor of recognizer.MatchDevfiles which additionally includes devfileFilters in order to filter the fetched stacks from the registry.

Along with code updates, changes were made in the tests and in the documentation.

Which issue(s) does this PR fix

fixes devfile/api#1173

PR acceptance criteria

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.

  • Unit/Functional tests

  • Documentation

How to test changes / Special notes to the reviewer

thepetk added 3 commits August 8, 2023 14:48
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
@thepetk thepetk requested a review from mike-hoang August 8, 2023 15:53
@thepetk thepetk self-assigned this Aug 8, 2023
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch coverage: 61.53% and no project coverage change.

Comparison is base (aa486b0) 66.56% compared to head (a924dc4) 66.56%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #18   +/-   ##
=======================================
  Coverage   66.56%   66.56%           
=======================================
  Files          10       10           
  Lines        1358     1358           
=======================================
  Hits          904      904           
  Misses        394      394           
  Partials       60       60           
Files Changed Coverage Δ
test/check_registry/check_registry.go 46.71% <0.00%> (ø)
pkg/apis/recognizer/devfile_recognizer.go 53.62% <62.33%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@openshift-ci
Copy link

openshift-ci bot commented Aug 8, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mike-hoang, thepetk

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@thepetk
Copy link
Contributor Author

thepetk commented Aug 9, 2023

As the PR is not introducing any new features is good to be merged. For the code coverage warnings I've added a new issue here: devfile/api#1218

@rm3l
Copy link
Member

rm3l commented Oct 23, 2023

FYI, this PR introduced breaking changes by renaming the exported model.DevFileType type (part of the Alizer API) into model.DevfileType.
I was wondering why a minor bump of Alizer from 1.0.1 to 1.2.1 (redhat-developer/odo#7126) was not compiling, and it appears it was due to this refactoring.

go install -mod=vendor -ldflags="-X github.com/redhat-developer/odo/pkg/version.GITCOMMIT=7b6096a6f" ./cmd/odo/
# github.com/redhat-developer/odo/pkg/alizer
pkg/alizer/interface.go:11:23: undefined: model.DevFileType
pkg/alizer/alizer.go:33:19: undefined: model.DevFileType
pkg/alizer/alizer.go:39:31: undefined: model.DevFileType
pkg/alizer/alizer.go:147:35: undefined: model.DevFileType
make: *** [Makefile:101: install] Error 2

As such, this PR should have been released as a major version, I think. Not a big deal that said, but just for awareness.

@thepetk
Copy link
Contributor Author

thepetk commented Oct 23, 2023

FYI, this PR introduced breaking changes by renaming the exported model.DevFileType type (part of the Alizer API) into model.DevfileType. I was wondering why a minor bump of Alizer from 1.0.1 to 1.2.1 (redhat-developer/odo#7126) was not compiling, and it appears it was due to this refactoring.

go install -mod=vendor -ldflags="-X github.com/redhat-developer/odo/pkg/version.GITCOMMIT=7b6096a6f" ./cmd/odo/
# github.com/redhat-developer/odo/pkg/alizer
pkg/alizer/interface.go:11:23: undefined: model.DevFileType
pkg/alizer/alizer.go:33:19: undefined: model.DevFileType
pkg/alizer/alizer.go:39:31: undefined: model.DevFileType
pkg/alizer/alizer.go:147:35: undefined: model.DevFileType
make: *** [Makefile:101: install] Error 2

As such, this PR should have been released as a major version, I think. Not a big deal that said, but just for awareness.

Yes I think this should be a major release in the future. Back then, we had only a bunch of releases of alizer. Now I think the situation is better, but for sure we need to treat breaking changes as major releases.

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.

Rename DevFile to Devfile inside alizer code
3 participants