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: support context file location in repository #567

Merged
merged 23 commits into from
Aug 7, 2023

Conversation

aeworxet
Copy link
Contributor

@aeworxet aeworxet commented May 19, 2023

This PR adds support of the context file's location in repository.

  • default context file's name was changed to .asyncapi-cli
  • getContextFilePath() searches for a context file named as specified in CONTEXT_FILENAME:
    • in current directory,
    • if it doesn't find, it goes upper, and upper, and repeats, up to the root of the repository,
    • if it still cannot find the context file, it searches in the user's home directory,
    • if there is no context file even there, the function stops execution, the context file's name is defaulting to be ${os.homedir()}/.asyncapi-cli and this path is assigned to CONTEXT_FILE_PATH which is then returned on every request.
  • getRepoRootPath() searches for a supposed root of the repository starting from current directory all the way up the filesystem, using presence of the .git directory as an indicator; if it doesn't find, then it is assumed that the root of the repository is the current directory
  • initContext() performs initialization of an empty context file with command asyncapi config context init in:
    • current directory: asyncapi config context init . (default)
    • root of current repository: asyncapi config context init ./
    • user's home directory: asyncapi config context init ~
  • editContext() performs edit of a specified context in context file's store (this functionality was previously handled by addContext() but possibility of edit was broken by introduction of the check if given context's name already exists in context file's store)
  • isContextFileValid() performs boolean validation of context file's format against interface IContextFile
  • isContextFileEmpty() performs boolean check if context file can be considered a minimal empty context file, according to https://www.asyncapi.com/docs/tools/cli/context#minimalEmptyContextFile
  • context files that have wrong format (didn't pass validation by isContextFileValid()) are ignored even if they have proper name
  • catch(error), catch(err) and catch(e) were brought to unified form
  • messages of existing errors and exceptions were made more meaningful and new ones were added
  • ContextNotFound was renamed to ContextNotFoundError, to keep error objects' naming in ./src/errors/context-error.ts in the same style
  • ./src/errors/context-error.ts was tidied up overall
  • minor typos in various outputs were fixed
  • all files related to Context functionality were formatted with Prettier
  • testing code was updated and new tests were added
  • file ./docs/context.md with documentation on Context functionality was added

Current @oclif/test's implementation leads to unexpected results in some scenarios, all observations were described in ./test/commands/context.test.ts as comments. Making fixes to @oclif/test is out of scope.

Resolves #38

@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch 3 times, most recently from e2f1bc2 to 0dcf5ea Compare June 2, 2023 12:03
@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch 2 times, most recently from 9f05f43 to 3e5e14d Compare June 3, 2023 01:01
@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch 13 times, most recently from 639d864 to ad39d0f Compare June 28, 2023 11:24
@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch from ad39d0f to 93dd030 Compare June 30, 2023 16:54
@aeworxet aeworxet marked this pull request as ready for review June 30, 2023 17:16
@derberg
Copy link
Member

derberg commented Jul 4, 2023

I tried manual testing


Current CLI behaviour

$ asyncapi config context current
ContextError: No context is set as current, please set a current context.
$ asyncapi config context list
myapp: /Users/wookiee/sources/cli/test/fixtures/asyncapi_v1.yml

PR behaviour - when I have no context

$ bin/run config context current
ContextError: These are your options to specify in the CLI what AsyncAPI file should be used:
        - You can provide a path to the AsyncAPI file: asyncapi <command> path/to/file/asyncapi.yml
        - You can provide URL to the AsyncAPI file: asyncapi <command> https://example.com/path/to/file/asyncapi.yml
        - You can also pass a saved context that points to your AsyncAPI file: asyncapi <command> context-name
        - In case you did not specify a context that you want to use, the CLI checks if there is a default context and uses it. To set default context run: asyncapi config context use mycontext
        - In case you did not provide any reference to AsyncAPI file and there is no default context, the CLI detects if in your current working directory you have files like asyncapi.json, asyncapi.yaml, asyncapi.yml. Just rename your file accordingly.
$ bin/run config context list
ContextError: These are your options to specify in the CLI what AsyncAPI file should be used:
        - You can provide a path to the AsyncAPI file: asyncapi <command> path/to/file/asyncapi.yml
        - You can provide URL to the AsyncAPI file: asyncapi <command> https://example.com/path/to/file/asyncapi.yml
        - You can also pass a saved context that points to your AsyncAPI file: asyncapi <command> context-name
        - In case you did not specify a context that you want to use, the CLI checks if there is a default context and uses it. To set default context run: asyncapi config context use mycontext
        - In case you did not provide any reference to AsyncAPI file and there is no default context, the CLI detects if in your current working directory you have files like asyncapi.json, asyncapi.yaml, asyncapi.yml. Just rename your file accordingly.

Expected behaviour - when I have no context

//no error but info
$ bin/run config context current
No context is set as current. Run 'asyncapi config context' to see all the available options.
//no error but info
$ bin/run config context list
You have no context configured. Run 'asyncapi config context' to see all the available options.

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

I'm missing some docs as well, but this is big task as we have no docs covering context, so I would say it is out of scope here. Please open a followup issue that we need to explain context concept, tips and tricks and also make sure we explain how to add context to a project, the file structure. Unless you think you can add something basic already

@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch from b29ede5 to e160a2f Compare July 11, 2023 09:22
@derberg derberg changed the title feat: support context file location in repository (#38) feat: support context file location in repository Jul 11, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

nice you're adding docs, but from my side - it was not in scope. Normally docs should come with implementation but I'm also honest admitting there were zero docs for the concept, so this is other case. I'm totally fine to extend time for this issue and its scope, but please be prepared that it will take more time to merge (you probably notices it after my comments, and there will be also another review later from docs owner).

so in other words

  • if you want to continue in this PR - I'm totally fine with it
  • if you want to open a separate PR - I'm totally fine with it

docs/context.md Outdated Show resolved Hide resolved
docs/context.md Outdated Show resolved Hide resolved
docs/context.md Outdated Show resolved Hide resolved
docs/context.md Outdated Show resolved Hide resolved
docs/context.md Outdated Show resolved Hide resolved
docs/context.md Outdated Show resolved Hide resolved
docs/context.md Show resolved Hide resolved
docs/context.md Outdated Show resolved Hide resolved
@aeworxet
Copy link
Contributor Author

nice you're adding docs, but from my side - it was not in scope.

After this moment the doc became intertwined with the code I wrote
#567 (comment)
and it is likely that changes to code will need to be introduced basing on the doc, so I'll need to steer both of them simultaneously.

It is a yet another Bounty Program usecase.
What should be the process of extending timeline for a Bounty task in case of unpredicted force majeure circumstances, which are beyond control of both contributor and maintainer and appeared in fact out of thin air?
@thulieblack

@thulieblack
Copy link
Member

We extend the deadline to fit the appropriate time to complete the issue. These are some things we are learning through this Bounty Trial, and since it is necessary to add documentation, we have to extend the deadline.

@aeworxet
Copy link
Contributor Author

Integration of context.md into website structure can be viewed at
https://8mnlsn-3000.csb.app/docs/tools/cli/context

@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch 5 times, most recently from 74f0199 to 95a0423 Compare July 26, 2023 08:04
@aeworxet aeworxet requested a review from derberg July 27, 2023 09:10
@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch 2 times, most recently from e110c3a to 64d570d Compare July 31, 2023 11:52
docs/context.md Outdated Show resolved Hide resolved
@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch from 856ddb6 to 7cbd02c Compare August 2, 2023 06:16
docs/context.md Outdated Show resolved Hide resolved
@aeworxet aeworxet force-pushed the make-contexts-shareable-#38 branch from 7cbd02c to 4575a70 Compare August 2, 2023 11:23
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

amazing work mate!!! 🚀

code + tests + docs => full stack ❤️

@Souvikns @magicmatatjahu do you guys plan to review? It is a big feature, so I hope not 😆

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.

LGTM just left a small suggestion.

src/commands/config/context/add.ts Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Aug 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@derberg
Copy link
Member

derberg commented Aug 7, 2023

/rtm

@asyncapi-bot asyncapi-bot merged commit e88fef0 into asyncapi:master Aug 7, 2023
11 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.52.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aeworxet aeworxet deleted the make-contexts-shareable-#38 branch August 9, 2023 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

Make contexts shareable
5 participants