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

Issues out of review of Projectile #1646

Closed
smiths opened this issue Jun 27, 2019 · 2 comments · Fixed by #1651
Closed

Issues out of review of Projectile #1646

smiths opened this issue Jun 27, 2019 · 2 comments · Fixed by #1651
Assignees

Comments

@smiths
Copy link
Collaborator

smiths commented Jun 27, 2019

@samm82, you have done a great job making the changes to Projectile based on my latest review, or creating issues out of my comments. However, there are a few comments that I don't recall seeing making it to issues, or where I have more to add. 😄 The right thing to do would be to search through the issues to see if I missed something, but I need to save time, so I'm just going to list the issues I think might be missing and then give you a chance to respond via this issue:

  1. When the SRS includes symbols that are vectors, I'd like the table of symbols preamble to include text that says something like: "For vector quantities, the units shown are for each component of the vector." You made this change for Projectile. Is it possible to make the change for the other examples that have vector symbols, or even better to automate this?
  2. For vector quantities we wanted their description to automatically add the word vector. Was there an issue created about this? (I vaguely recall that there might have been.)
  3. You made the change to the definition of Cartesian coordinate system. I like the change, but now I feel like we should add a reference to Wikipedia, since this is the source of our inspiration. Is it possible to add references to terminology?
  4. Do we have an issue for the text in IM:messageIM? The spacing of the strings makes them almost unreadable.
  5. Do we have an issue for moving Table 5 (Latex version numbering) into section 3.2.7 (Latex version numbering)? I think we should make that move for all of the examples. The output constraints are properties of the correct solution.
  6. In Table 10 (LaTeX version), g should be defined as the approximate acceleration due to gravity on Earth at sea level. The value of 9.8 doesn't apply at other elevations and at other heights. Thinking about it now, we should add another assumption - The projectile system is located on Earth at sea level.
@samm82
Copy link
Collaborator

samm82 commented Jun 28, 2019

  1. I made a docLang constructor for the sentence, so it would be trivial to add it to all the other examples. However, we don't automate the tables for what introduction sentences they should have, so I don't think this is worth focusing on right now.
  2. Extract Type Information from Symbol #1558
  3. I think we can - I'll go ahead on it!
  4. I think that would be part of Display Variable Names Better in LaTeX #1639. Never mind, it's a separate issue. Working on it now!
  5. Move Output Data Constraints to Properties of Correct Solution #1624
  6. Changing the definition of g would change it in other examples as well. Is that OK? @smiths I also think that this is related to Add constants to Drasil-Data #1592, as the constant for g is currently defined in Projectile and should be pulled out at some point. EDIT: Moved to Add constants to Drasil-Data #1592.

@samm82
Copy link
Collaborator

samm82 commented Jun 28, 2019

Every issue has either been fixed in #1651 or been addressed in other issues. Once #1651 is merged, this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants