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

use more strict formatting #385

Merged
merged 3 commits into from
Jul 20, 2022

Conversation

wilsonwang371
Copy link
Collaborator

@wilsonwang371 wilsonwang371 commented Jul 18, 2022

Why are these changes needed?

Use a more strict format check with gofumpt

We discussed this a long time before. Due to some conflicts with our internal branch, we postponed this until now. Since our downstream branch and upstream branch is in sync now, it is a good time for us to apply a more strict code formatting on kuberay.

Related issue number

N/A

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@wilsonwang371
Copy link
Collaborator Author

wilsonwang371 commented Jul 18, 2022

//key, _ := cmd.Flags().GetString("key")
//value, _ := cmd.Flags().GetString("value")
// key, _ := cmd.Flags().GetString("key")
// value, _ := cmd.Flags().GetString("value")
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, that's great

Copy link
Collaborator

@DmitriGekhtman DmitriGekhtman left a comment

Choose a reason for hiding this comment

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

Great! The less we have to think about format, the better :)

@DmitriGekhtman
Copy link
Collaborator

@wilsonwang371
You might want to give the collab slack channel a heads up, just in case.


- name: Open this to see how to fix gofumpt if it fails
run: |
echo "Run command 'gofumpt -w apiserver/ ray-operator/ cli/' to correct your code format."
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: will this check correct the import format? for example:
https://github.com/ray-project/kuberay/blob/master/ray-operator/controllers/ray/raycluster_controller.go#L3-L37

import (
	"context"
	"fmt"
	"strings"
	"time"

	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

	"github.com/ray-project/kuberay/ray-operator/controllers/ray/common"
	"github.com/ray-project/kuberay/ray-operator/controllers/ray/utils"

	rbacv1 "k8s.io/api/rbac/v1"

	rayiov1alpha1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1alpha1"
	"k8s.io/client-go/tools/record"

	"github.com/go-logr/logr"
	_ "k8s.io/api/apps/v1beta1"

	corev1 "k8s.io/api/core/v1"
	v1 "k8s.io/api/core/v1"
	networkingv1 "k8s.io/api/networking/v1"
	"k8s.io/apimachinery/pkg/api/errors"
	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/apimachinery/pkg/types"
	ctrl "sigs.k8s.io/controller-runtime"
	"sigs.k8s.io/controller-runtime/pkg/client"
	controller "sigs.k8s.io/controller-runtime/pkg/controller"
	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
	"sigs.k8s.io/controller-runtime/pkg/handler"
	logf "sigs.k8s.io/controller-runtime/pkg/log"
	"sigs.k8s.io/controller-runtime/pkg/manager"
	"sigs.k8s.io/controller-runtime/pkg/reconcile"
	"sigs.k8s.io/controller-runtime/pkg/source"
)

would need some format

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it will check the imports but the code you show will pass the check.

Copy link
Collaborator

Choose a reason for hiding this comment

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

got it, it seems like some codes writing practice issues and gofumpt may not correct them, in this case we may need to modify manually.

Copy link
Collaborator

@scarlet25151 scarlet25151 left a comment

Choose a reason for hiding this comment

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

LGTM.

@wilsonwang371 wilsonwang371 force-pushed the wilson/strict-formatting branch 2 times, most recently from ef505c7 to 1f876b2 Compare July 19, 2022 20:22
@Jeffwan
Copy link
Collaborator

Jeffwan commented Jul 20, 2022

@wilsonwang371 Let's rebase the change and we can unblock the PR.

@Jeffwan Jeffwan merged commit c9c89a5 into ray-project:master Jul 20, 2022
@wilsonwang371 wilsonwang371 deleted the wilson/strict-formatting branch July 20, 2022 20:40
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
* use more strict formatting

* rebase master

* update makefile
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.

None yet

4 participants