Skip to content

ITP-JAN-LONDON | Bakytbek Sydykov | Module-Structuring-and-Testing-Data | sprint 1#291

Closed
Bahasyd wants to merge 13 commits intoCodeYourFuture:mainfrom
Bahasyd:acourswork-sprint-1
Closed

ITP-JAN-LONDON | Bakytbek Sydykov | Module-Structuring-and-Testing-Data | sprint 1#291
Bahasyd wants to merge 13 commits intoCodeYourFuture:mainfrom
Bahasyd:acourswork-sprint-1

Conversation

@Bahasyd
Copy link

@Bahasyd Bahasyd commented Feb 15, 2025 •
edited
Loading

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

n/c

Questions

Ask any questions you have for your reviewer.

@Bahasyd Bahasyd added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 15, 2025
@cjyuan cjyuan added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Feb 22, 2025
cjyuan
cjyuan reviewed Feb 22, 2025
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Very clear explanation!

I think you missed 3-mandatory-interpret/2-time-format.js and 1-key-exercises/4-random.js.

Why not try also the questions in 4-stretch-explore/?

Sprint-1/2-mandatory-errors/2.js
console.log(`I was born in ${cityOfBirth}`);
const cityOfBirth = "Bolton";

// "const" must be before "console" No newline at end of file
Copy link
Contributor

@cjyuan cjyuan Feb 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Your description is a bit vague. Can you rephrase it using the proper programming terms?

Note: What is the programming term that refers to a statement that introduces new variables into a program?

Sprint-1/3-mandatory-interpret/3-to-pounds.js

substring(1)
Extracts the string starting from index 1 — we get "99".
padEnd(2, "0") pads the string with zeros up to 2 characters, if necessary.
Copy link
Contributor

@cjyuan cjyuan Feb 22, 2025

Choose a reason for hiding this comment

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

Can we expect this program to work as intended even if we deleted .padEnd(2, "0") from the code?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Feb 22, 2025
@Bahasyd Bahasyd added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 24, 2025
cjyuan
cjyuan reviewed Mar 4, 2025
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Sprint-1/1-key-exercises/4-random.js still appears to be unmodified.

The new addition, Sprint-1/3-mandatory-interpret/2-time-format.js, looks good.

How come in this Sprint-1 branch there are modified files in "Sprint-2" and "Sprint-3" folder?

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 4, 2025
@Bahasyd Bahasyd self-assigned this Mar 10, 2025
@Bahasyd Bahasyd added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Mar 10, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Mar 10, 2025

It doesn't seem you have made any changes since my last review on 4 Mar.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 10, 2025
@Bahasyd
Copy link
Author

Bahasyd commented Mar 10, 2025

Sprint-1/1-key-exercises/4-random.js still appears to be unmodified.
I've done it.
The new addition, Sprint-1/3-mandatory-interpret/2-time-format.js, looks good.
Thank you.
How come in this Sprint-1 branch there are modified files in "Sprint-2" and "Sprint-3" folder?
I say sorry. I cleaned everything and left it as it was originally.

@Bahasyd Bahasyd added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 10, 2025
@Bahasyd Bahasyd added the Complete Volunteer to add when work is complete and all review comments have been addressed. label Mar 10, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Mar 11, 2025

  1. The changes in Sprint-1/1-key-exercises/4-random.js looks good.

  2. Only one of these labels, "Needs Review" and "Completed", should be added in the last step of a review process (according to the review process described in https://codeyourfutur-yov6609.slack.com/canvas/C07QX99JK7B).

  3. The modified files still show up in the Sprint-2 and Sprint-3 folders. You can see them in the "Files changed" tab in your PR.
    image

Git uses "difference in content" and not timestamp to identify modified files. So one way to make the files not showing up as modified files is to replace them by the original files (those in the main branch).

@Bahasyd Bahasyd removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjyuan cjyuan cjyuan left review comments

Assignees

@Bahasyd Bahasyd

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Bahasyd @cjyuan @illicitonion