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

feat: improve glee command #890

Merged
merged 65 commits into from
Feb 1, 2024
Merged

Conversation

ayushnau
Copy link
Contributor

@ayushnau ayushnau commented Nov 18, 2023

  • Fetched all the operations and created a function for them.
  • Updated the asyncapi.yaml file with the use provided.
  • used the modelina to create the types for the function.
  • updated the server type in the env.
image image

Resolves #683

@Souvikns Souvikns changed the title Feature improve glee command feat: improve glee command Nov 29, 2023
Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

./bin/run new glee works and it generates all the files, but when I try to generate from a file using ./bin/run new glee -f asyncapi.yaml then it reads the file and asks me to select the servers but it fails to create all the necessary files. It is creating an empty project folder.

export default class NewGlee extends Command {
static description = 'Creates a new Glee project';

protected commandName = 'glee';

static flags = {
help: Flags.help({ char: 'h' }),
name: Flags.string({ char: 'n', description: 'name of the project', default: 'project' }),
template: Flags.string({ char: 't', description: 'name of the template', default: 'default' }),
name: Flags.string({
Copy link
Member

Choose a reason for hiding this comment

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

unable to regenerate the glee project as it is missing the --force-write flag.

Screenshot 2024-01-23 at 7 38 56 PM Screenshot 2024-01-23 at 7 39 03 PM

Copy link
Member

Choose a reason for hiding this comment

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

it is a requirement by the generator. you can't generate on a git repository with upstaged changes.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but can't we pass in the flag from our CLI, since generate fromTemplate does that?

Copy link
Member

Choose a reason for hiding this comment

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

@Souvikns I think it is best to expose --force-write in asyncapi new glee command. and leave it to the user if they want to force write or not. 🤷

@ayushnau can you add --force-write flag to asyncapi new glee command and then pass it on to the generator if user passes it? 🤔

Choose a reason for hiding this comment

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

ok sure will do that.

Copy link

Choose a reason for hiding this comment

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

@KhudaDad414 Added the required changes. please review.

Choose a reason for hiding this comment

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

/rtm

Choose a reason for hiding this comment

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

@KhudaDad414 i have Added force-write flag. please review.

Copy link
Member

@peter-rr peter-rr Jan 29, 2024

Choose a reason for hiding this comment

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

@AyushNautiyalDeveloper
--force-write flag addition LGTM 👍

peter-rr

This comment was marked as duplicate.

Copy link
Member

@peter-rr peter-rr left a comment

Choose a reason for hiding this comment

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

@AyushNautiyalDeveloper Have you considered to add some tests mostly for the new functions added?

@AyushNautiyalDeveloper
Copy link

@peter-rr can add but can we merge this before that.

Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

I am getting this error when I run the ./bin/run new glee -f asyncapi.yaml , @AyushNautiyalDeveloper can you check it once

Screenshot 2024-01-31 at 1 00 41 PM

@AyushNautiyalDeveloper
Copy link

I am getting this error when I run the ./bin/run new glee -f asyncapi.yaml , @AyushNautiyalDeveloper can you check it once

Screenshot 2024-01-31 at 1 00 41 PM

can you share the file you are using.

@Souvikns
Copy link
Member

I am getting this error when I run the ./bin/run new glee -f asyncapi.yaml , @AyushNautiyalDeveloper can you check it once
Screenshot 2024-01-31 at 1 00 41 PM

can you share the file you are using.

I don't think the file is incorrect I double-checked it in the studio as well, it is the file that you get from running the command ./bin/run new file

asyncapi: 3.0.0
info:
  title: Account Service
  version: 1.0.0
  description: This service is in charge of processing user signups
channels:
  userSignedUp:
    address: user/signedup
    messages:
      UserSignedUp:
        $ref: '#/components/messages/UserSignedUp'
operations:
  onUserSignUp:
    action: receive
    channel:
      $ref: '#/channels/userSignedUp'
    messages:
      - $ref: '#/channels/userSignedUp/messages/UserSignedUp'
components:
  messages:
    UserSignedUp:
      payload:
        type: object
        properties:
          displayName:
            type: string
            description: Name of the user
          email:
            type: string
            format: email
            description: Email of the user

@AyushNautiyalDeveloper
Copy link

@Souvikns can you check now.

@peter-rr
Copy link
Member

@peter-rr can add but can we merge this before that.

Ok, please double check that with @Souvikns or @KhudaDad414 as well :)

@AyushNautiyalDeveloper
Copy link

@peter-rr can add but can we merge this before that.

Ok, please double check that with @Souvikns or @KhudaDad414 as well :)

ok sure.

@aeworxet
Copy link
Contributor

aeworxet commented Feb 1, 2024

@AyushNautiyalDeveloper
There are 2 New issues reported in the Quality Gate: 'path' imported multiple times.
They are minor code smells, but still.

You can replace

import { resolve, join } from 'path';
import path from 'path';

with

import path, { resolve, join } from 'path';

to keep sonarcloud happy.

@AyushNautiyalDeveloper
Copy link

@AyushNautiyalDeveloper There are 2 New issues reported in the Quality Gate: 'path' imported multiple times. They are minor code smells, but still.

You can replace

import { resolve, join } from 'path';
import path from 'path';

with

import path, { resolve, join } from 'path';

to keep sonarcloud happy.

done @aeworxet

Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

Yeah, it is working now, I suggest you open a new PR with tests. Good Job @AyushNautiyalDeveloper

Copy link

sonarqubecloud bot commented Feb 1, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Souvikns
Copy link
Member

Souvikns commented Feb 1, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit 6cfd356 into asyncapi:master Feb 1, 2024
10 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@AyushNautiyalDeveloper
Copy link

Yeah, it is working now, I suggest you open a new PR with tests. Good Job @AyushNautiyalDeveloper

thanks. @Souvikns

@asyncapi-bot asyncapi-bot added the bounty AsyncAPI Bounty program related label label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty AsyncAPI Bounty program related label ready-to-merge released
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

Improve new glee command
8 participants