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 workflow folder and Fix the Dockerfile syntax #20

Merged
merged 6 commits into from
Aug 5, 2024

Conversation

x86taka
Copy link
Collaborator

@x86taka x86taka commented Jul 17, 2024

What

  • fix: Rename a GitHubActions workflow folder
  • lint: fix lowercase

Why

Merge condition

@x86taka x86taka self-assigned this Jul 17, 2024
@x86taka x86taka added the bug Something isn't working label Jul 23, 2024
@x86taka x86taka requested a review from takehaya July 23, 2024 19:55
m.NewAVP(avp.SessionID, avp.Mbit, 0, datatype.UTF8String(sid))
m.NewAVP(avp.OriginHost, avp.Mbit, 0, c.cfg.OriginHost)
m.NewAVP(avp.OriginRealm, avp.Mbit, 0, c.cfg.OriginRealm)
_, err = m.NewAVP(avp.SessionID, avp.Mbit, 0, datatype.UTF8String(sid))
Copy link
Member

Choose a reason for hiding this comment

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

Adding if err != nil every time we increase the code for NewAVP reduces readability.
It would be great if you could simplify it like this.

for example.

type avpData struct {
    code   avp.Code
    flag   avp.Flag
    vendor uint32
    value  datatype.Type
}

avps := []avpData{
    {avp.SessionID, avp.Mbit, 0, datatype.UTF8String(sid)},
    {avp.OriginHost, avp.Mbit, 0, c.cfg.OriginHost},
    {avp.OriginRealm, avp.Mbit, 0, c.cfg.OriginRealm},
}

for _, avp := range avps {
    if _, err := m.NewAVP(avp.code, avp.flag, avp.vendor, avp.value); err != nil {
        return false, errors.WithMessagef(err, "NewAVP %v failed", avp.code) // fix format me
    }
}

if options.DestinationHost == nil {
m.NewAVP(avp.DestinationHost, avp.Mbit, 0, meta.OriginHost)
_, err := m.NewAVP(avp.DestinationHost, avp.Mbit, 0, meta.OriginHost)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@@ -0,0 +1,59 @@
run:
timeout: 15m
skip-files:
Copy link
Member

Choose a reason for hiding this comment

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

I think this was taken from another project. so plz do not include it.

@x86taka x86taka requested a review from takehaya August 5, 2024 04:12
Copy link
Member

@takehaya takehaya left a comment

Choose a reason for hiding this comment

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

:lgtm:

@takehaya takehaya merged commit 6524acb into master Aug 5, 2024
2 checks passed
@takehaya takehaya deleted the chore/github-workflows branch August 5, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants