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.

Sign up for GitHub

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

Jump to bottom

chore: refactor the generator implementation #2171

Merged
merged 108 commits into from
Jun 17, 2024
Merged

chore: refactor the generator implementation #2171

merged 108 commits into from
Jun 17, 2024

Conversation

laststylebender14
Copy link
Contributor

@laststylebender14 laststylebender14 commented Jun 11, 2024 •
edited
Loading

Summary:

  • refactored generalised config generation code.

Issue Reference(s):
Fixes #... (Replace "..." with the issue number)

Build & Testing:

  • I ran cargo test successfully.
  • I have run ./lint.sh --mode=fix to fix all linting issues raised by ./lint.sh --mode=check.

Checklist:

  • I have added relevant unit & integration tests.
  • I have updated the documentation accordingly.
  • I have performed a self-review of my code.
  • PR follows the naming convention of <type>(<optional scope>): <title>

@laststylebender14
- added support to detect if import src is url or not.
1f148ac
@laststylebender14
issue: last generated config ends up overriding the previously genera…
d3b8b26 
…ted configs.

fix: moved name generators to top.
@laststylebender14
- export name generator for tests.
7f76b9a
@laststylebender14
- fix from json.
e075b17
@laststylebender14
Merge branch 'main' into feat/add-support-for-rest-to-config-gen
0f4a54d
@laststylebender14
- added path-clean crate to clean the urls (remove ../../ or .).
9a4c8f7
@laststylebender14
- resolve the relative paths correctly for input as well as output.
dfa3620
@laststylebender14
- instead using absolute path use relative path.
01a0fef
@laststylebender14
- resolve paths for both input source: config and import.
0c4fb07
@laststylebender14
- added test case and snap.
04a5061
@laststylebender14
- if output file already exists, confirm with user for overwriting.
c79cc17
@laststylebender14
- maintain src as well as resolved_src.
b621c37 
- use resolved_src for reading file or remote resource but for linking use src.
@laststylebender14
- updated snaps.
63febd6
@laststylebender14
- use unwrap_or instead of if-else.
33dccdc
@laststylebender14
- use resolved_src in source config as well.
a0818bd
@laststylebender14
- refactored: removed duplicate code.
ed860df
@laststylebender14
Merge branch 'feat/generalized-config-gen' into feat/add-support-for-…
e6609c7 
…rest-to-config-gen
@laststylebender14
- added unit tests for ImportSource::detect.
2fee41d
@laststylebender14
- use url parse instead of regex.
819d42a
@laststylebender14
- handle ends_with better way.
59b635f
@laststylebender14
Merge branch 'feat/add-support-for-rest-to-config-gen' into feat/reso…
a8f4e30 
…lve-paths-inside-the-config
@laststylebender14
- refactor: adapted new design to generate the tailcall config from g…
a741838 
…eneralized config json.
@laststylebender14
- use FromGeneralizedConfig in tc.
38d7181
@laststylebender14 laststylebender14 marked this pull request as draft June 11, 2024 18:49
@github-actions github-actions bot added the type: chore Routine tasks like conversions, reorganization, and maintenance work. label Jun 11, 2024
@github-actions github-actions bot removed the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Jun 17, 2024
tusharmath
tusharmath reviewed Jun 17, 2024
View reviewed changes
src/core/generator/generator.rs Outdated
operation_name: Option<String>,
proto_samples: Option<Vec<ProtoInput>>,
json_samples: Option<Vec<JsonInput>>,
config_samples: Option<Vec<ConfigInput>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Config is not a sample. Rename the field.

Copy link
Contributor

Choose a reason for hiding this comment

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

Create a Vec<Input> where Input is an Enum. Keep proto, json, config all as inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

tusharmath
tusharmath reviewed Jun 17, 2024
View reviewed changes
src/core/generator/generator.rs Outdated
reader: ResourceReader<Cached>,
proto_reader: ProtoReader,
operation_name: Option<String>,
inputs: Option<Vec<Input>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Option isn't required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed optional.

@tusharmath tusharmath merged commit 9489120 into feat/generalized-config-gen Jun 17, 2024
6 checks passed
@tusharmath tusharmath deleted the chore/refactor-the-generator-implementation branch June 17, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tusharmath tusharmath tusharmath left review comments

Assignees
No one assigned
Labels
ci: benchmark Runs benchmarks type: chore Routine tasks like conversions, reorganization, and maintenance work.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@laststylebender14 @tusharmath