Type annotations #41

yoonieaj · 2024-06-04T18:28:12Z

Proposed Changes

Added type interfaces and type annotations for functions and constants in style.ts. This should improve code organisation/readability.
...

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)	x
🚦 Test update (change that only adds or modifies tests)
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

I have performed a self-review of my changes.
- Check that all changed files included in this pull request are intentional changes.
- Check that all changes are relevant to the purpose of this pull request, as described above.
I have added tests for my changes, if applicable.
- This is required for all bug fixes and new features.
I have updated the project documentation, if applicable.
- This is required for new features.
If this is my first contribution, I have added myself to the list of contributors.
I have updated the project Changelog (this is required for all changes).

After opening your pull request:

I have verified that the CI tests have passed.
I have reviewed the test coverage changes reported by Coveralls.
I have requested a review from a project maintainer.

Questions and Comments

Hello Professor Liu! I have a few questions about the changes I've made so far:

The code doesn't seem to care whether the DrawnObject attributes are optional, so I'm not sure if I made the right call on which attributes should be optional?
Since the preset Styles only specify certain attributes, can I leave every one of these attributes as optional?

…nto type-annotations

…render blank objects

coveralls · 2024-06-04T18:39:23Z

Pull Request Test Coverage Report for Build 9372440637

Details

16 of 16 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 86.781%

Totals
Change from base Build 9327208107:	0.0%
Covered Lines:	380
Relevant Lines:	423

💛 - Coveralls

david-yz-liu · 2024-06-05T00:52:05Z

Weird that the compiler doesn't seem to be catching that, but we can look into that later. I think show_indexes and style can both be optional as well.
Yes it's good for all Style attributes to be optional 👍

…-viz into type-annotations

coveralls · 2024-06-05T19:15:21Z

Pull Request Test Coverage Report for Build 9389923649

Details

17 of 17 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 86.801%

Totals
Change from base Build 9376313216:	0.0%
Covered Lines:	381
Relevant Lines:	424

💛 - Coveralls

yoonieaj · 2024-06-05T19:27:21Z

Proposed Changes

Added type interfaces and type annotations for functions and constants in style.ts. This should improve code organisation/readability.
...

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)	x
🚦 Test update (change that only adds or modifies tests)
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

I have performed a self-review of my changes.
- Check that all changed files included in this pull request are intentional changes.
- Check that all changes are relevant to the purpose of this pull request, as described above.
I have added tests for my changes, if applicable.
- This is required for all bug fixes and new features.
I have updated the project documentation, if applicable.
- This is required for new features.
If this is my first contribution, I have added myself to the list of contributors.
I have updated the project Changelog (this is required for all changes).

After opening your pull request:

I have verified that the CI tests have passed.
I have reviewed the test coverage changes reported by Coveralls.
I have requested a review from a project maintainer.

Questions and Comments

(Include any questions or comments you have regarding your changes.)

coveralls · 2024-06-05T19:29:26Z

Pull Request Test Coverage Report for Build 9390090716

Details

17 of 17 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 86.801%

Totals
Change from base Build 9376313216:	0.0%
Covered Lines:	381
Relevant Lines:	424

💛 - Coveralls

david-yz-liu

@yoonieaj great work. I left a few inline comments. Also:

I see that you left a comment with your full PR description. Instead, you can edit your existing PR description and then copy back in the contents of that comment.
Since you've defined the DrawnObject type very well, please go ahead and modify the other source code files to use this type annotation instead of the old "object" type. You don't need to add type annotations to all functions/values in those other files though, just make sure to use DrawnObject where appropriate.

david-yz-liu · 2024-06-05T23:39:12Z

memory-viz/src/automate.ts

@@ -73,7 +73,7 @@ function drawAutomatedStackFrames(stack_frames, configuration) {
        let width;
        let height;

-        if (stack_frame.name !== "BLANK") {
+        if (stack_frame.type !== "BLANK") {


This change is out of scope of this PR (and in fact I suspect it's related to what @sarahsonder is working on currently).

david-yz-liu · 2024-06-05T23:40:24Z

memory-viz/src/types.ts

@@ -0,0 +1,25 @@
+interface DrawnObject {


Use the export keyword for these types, and then import them explicitly in style.ts.

Also rename this type to DrawnEntity, since the word "object" has another meaning in the context of memory model diagrams.

…nto type-annotations

coveralls · 2024-06-11T01:20:15Z

Pull Request Test Coverage Report for Build 9457996657

Details

15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 87.596%

Totals
Change from base Build 9410692102:	0.0%
Covered Lines:	388
Relevant Lines:	429

💛 - Coveralls

coveralls · 2024-06-11T01:22:15Z

Pull Request Test Coverage Report for Build 9458015375

Details

15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 87.596%

Totals
Change from base Build 9410692102:	0.0%
Covered Lines:	388
Relevant Lines:	429

💛 - Coveralls

david-yz-liu · 2024-06-11T21:02:41Z

memory-viz/src/types.ts

+    value: any;
+    show_indexes?: boolean;
+    style?: Style;
+    height: number;


I think this, width, and rowBreaker are also optional?

david-yz-liu · 2024-06-11T21:03:10Z

memory-viz/src/automate.ts

@@ -244,7 +245,7 @@ function drawAutomatedOtherItems(
 * The returned object has two attributes as 'stack_frames' and 'other_items'.
 * Each of these attributes are a list of objects that were originally given by the user.
 *
- * @param {object[]} objects - The list of objects, including stack-frames (if any) and other items, that
+ * @param {Object[]} objects - The list of objects, including stack-frames (if any) and other items, that


Is this meant to be DrawnEntity?

coveralls · 2024-06-11T23:24:30Z

Pull Request Test Coverage Report for Build 9474095102

Details

15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 87.596%

Totals
Change from base Build 9410692102:	0.0%
Covered Lines:	388
Relevant Lines:	429

💛 - Coveralls

yoonieaj added 4 commits May 31, 2024 13:41

add type annotations and interfaces

ae40d37

Merge branch 'master' of https://github.com/david-yz-liu/memory-viz i…

4c1bdd1

…nto type-annotations

Make relevant attributes optional and update automate.ts to properly …

e0e05cb

…render blank objects

Move type interfaces into a new file

4ad4b47

yoonieaj marked this pull request as ready for review June 4, 2024 18:37

yoonieaj marked this pull request as draft June 4, 2024 18:39

yoonieaj requested a review from david-yz-liu June 4, 2024 18:39

yoonieaj and others added 3 commits June 5, 2024 11:15

Make show_indexes and style attributes optional

8415a58

Update changelog

403cc5c

Merge branch 'master' into type-annotations

e4a3562

yoonieaj marked this pull request as ready for review June 5, 2024 19:13

Merge branch 'type-annotations' of https://github.com/yoonieaj/memory…

6fbea06

…-viz into type-annotations

yoonieaj marked this pull request as draft June 5, 2024 19:16

yoonieaj marked this pull request as ready for review June 5, 2024 19:27

david-yz-liu reviewed Jun 5, 2024

View reviewed changes

yoonieaj added 6 commits June 6, 2024 01:32

Rename DrawnObject to DrawnEntity and fix imports/exports

1f3ee04

Add DrawnEntity type annotation to source code files

a17f872

Merge branch 'master' of https://github.com/david-yz-liu/memory-viz i…

d6be266

…nto type-annotations

Remove isClass and stack_frame attributes from DrawnEntity

8e08aad

Add type annotations to source files

bd91cdd

Update changelog

07846c7

yoonieaj requested a review from david-yz-liu June 11, 2024 01:22

david-yz-liu reviewed Jun 11, 2024

View reviewed changes

Make new DrawnEntity attributes optional and fix annotation in types.ts

fef988a

yoonieaj requested a review from david-yz-liu June 11, 2024 23:28

david-yz-liu approved these changes Jun 12, 2024

View reviewed changes

david-yz-liu merged commit 399a80e into david-yz-liu:master Jun 12, 2024
3 checks passed

yoonieaj deleted the type-annotations branch September 17, 2024 00:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Type annotations #41

Type annotations #41

yoonieaj commented Jun 4, 2024 •

edited

Loading

coveralls commented Jun 4, 2024 •

edited

Loading

david-yz-liu commented Jun 5, 2024

coveralls commented Jun 5, 2024 •

edited

Loading

yoonieaj commented Jun 5, 2024 •

edited

Loading

coveralls commented Jun 5, 2024 •

edited

Loading

david-yz-liu left a comment

david-yz-liu Jun 5, 2024

david-yz-liu Jun 5, 2024

david-yz-liu Jun 5, 2024

coveralls commented Jun 11, 2024 •

edited

Loading

coveralls commented Jun 11, 2024 •

edited

Loading

david-yz-liu Jun 11, 2024

david-yz-liu Jun 11, 2024

coveralls commented Jun 11, 2024 •

edited

Loading

Type annotations #41

Type annotations #41

Conversation

yoonieaj commented Jun 4, 2024 • edited Loading

Proposed Changes

Type of Change

Checklist

Questions and Comments

coveralls commented Jun 4, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9372440637

Details

💛 - Coveralls

david-yz-liu commented Jun 5, 2024

coveralls commented Jun 5, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9389923649

Details

💛 - Coveralls

yoonieaj commented Jun 5, 2024 • edited Loading

Proposed Changes

Type of Change

Checklist

Questions and Comments

coveralls commented Jun 5, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9390090716

Details

💛 - Coveralls

david-yz-liu left a comment

Choose a reason for hiding this comment

david-yz-liu Jun 5, 2024

Choose a reason for hiding this comment

david-yz-liu Jun 5, 2024

Choose a reason for hiding this comment

david-yz-liu Jun 5, 2024

Choose a reason for hiding this comment

coveralls commented Jun 11, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9457996657

Details

💛 - Coveralls

coveralls commented Jun 11, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9458015375

Details

💛 - Coveralls

david-yz-liu Jun 11, 2024

Choose a reason for hiding this comment

david-yz-liu Jun 11, 2024

Choose a reason for hiding this comment

coveralls commented Jun 11, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9474095102

Details

💛 - Coveralls

yoonieaj commented Jun 4, 2024 •

edited

Loading

coveralls commented Jun 4, 2024 •

edited

Loading

coveralls commented Jun 5, 2024 •

edited

Loading

yoonieaj commented Jun 5, 2024 •

edited

Loading

coveralls commented Jun 5, 2024 •

edited

Loading

coveralls commented Jun 11, 2024 •

edited

Loading

coveralls commented Jun 11, 2024 •

edited

Loading

coveralls commented Jun 11, 2024 •

edited

Loading